Re: [PATCH v14 08/40] refs.c: add an err argument to delete_ref_loose

Ronnie Sahlberg wrote:

quoted

--- a/refs.c
+++ b/refs.c

@@ -2510,16 +2510,38 @@ static int repack_without_ref(const char *refname)

[...]

quoted

+static int unlink_or_err(const char *file, struct strbuf *err)
+{
+     if (err)
+             return add_err_if_unremovable("unlink", file, err,
+                                           unlink(file));
+     else
+             return unlink_or_warn(file);
+}

This has two different return value conventions (my fault, I know): it
returns -1 for ENOENT when err == NULL but 0 for ENOENT when not.  The
usual reason to call unlink_or_warn / remove_or_warn is to be silent
about ENOENT so it seems simplest to fix them at the same time, as
below.  The only callers that could mind:

 * unpack-trees.c::unlink_entry, where this would be fixing a minor bug
 * builtin/apply.c::remove_file, likewise
 * delete_ref_loose, which checks errno so this should have no effect there

 refs.c    |  2 +-
 wrapper.c | 14 ++++++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git i/refs.c w/refs.c
index dd498cf..e32aa97 100644
--- i/refs.c
+++ w/refs.c

@@ -2541,7 +2541,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
                lock->lk->filename[i] = 0;
                res = unlink_or_err(lock->lk->filename, err);
                lock->lk->filename[i] = '.';
-               if (res && errno != ENOENT)
+               if (res)
                        return 1;
        }
        return 0;

diff --git i/wrapper.c w/wrapper.c
index bc1bfb8..c9605cd 100644
--- i/wrapper.c
+++ w/wrapper.c

@@ -429,14 +429,12 @@ int xmkstemp_mode(char *template, int mode)

 static int warn_if_unremovable(const char *op, const char *file, int rc)
 {
-       if (rc < 0) {
-               int err = errno;
-               if (ENOENT != err) {
-                       warning("unable to %s %s: %s",
-                               op, file, strerror(errno));
-                       errno = err;
-               }
-       }
+       int err;
+       if (!rc || errno == ENOENT)
+               return 0;
+       err = errno;
+       warning("unable to %s %s: %s", op, file, strerror(errno));
+       errno = err;
        return rc;
 }