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

[Jonathan Nieder <jrnieder@gmail.com>]

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;
 }