Re: [PATCH v8 06/44] refs.c: add an err argument ro delete_loose_ref

On Thu, May 15, 2014 at 12:04 PM, Jonathan Nieder [off-list ref] wrote:

Ronnie Sahlberg wrote:

quoted

[Subject: refs.c: add an err argument ro delete_loose_ref]

s/ro/to/
s/delete_loose_ref/delete_ref_loose/

quoted

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

@@ -2484,17 +2484,22 @@ static int repack_without_ref(const char *refname)
      return repack_without_refs(&refname, 1, NULL);
 }

-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)

Should this get an onerr flag to suppress the message to stderr
or unconditionally suppress it when err != NULL?

Fixed.
I added a new function unlink_or_err that will update err if non-NULL
and unse warning() otherwise.

[...]

quoted

              lock->lk->filename[i] = 0;
-             err = unlink_or_warn(lock->lk->filename);
+             res = unlink_or_warn(lock->lk->filename);

It seems like in the new error handling scheme there should be a new
variant on wrapper.c's warn_if_unremovable:

        static int add_err_if_unremovable(const char *op, const char *file, struct strbuf *err, int rc)
        {
                int err = errno;
                if (rc < 0 && err != ENOENT) {
                        strbuf_addf(err, "unable to %s %s: %s",
                                    op, file, strerror(errno));
                        errno = err;
                }
                return rc;
        }

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

                res = unlink_or_err(lock->lk->filename, err);