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

> Ronnie Sahlberg wrote:
>
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2510,16 +2510,38 @@ static int repack_without_ref(const char *refname)
> [...]
>> +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;
>  }
>