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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 15, 2014, 19:04 UTC
Message-ID: <20140515190434.GF26471@google.com>
In-Reply-To: <1400174999-26786-7-git-send-email-sahlberg@google.com>

Ronnie Sahlberg wrote:

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

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

Show 8 quoted lines

> --- 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?

[...]

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