Re: [PATCH v14 08/40] refs.c: add an err argument to delete_ref_loose
From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:01:34
Ronnie Sahlberg wrote:
quoted hunk ↗ jump to hunk
--- 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; }