Re: [PATCH v9 1/7] refs.c: add err arguments to reflog functions

[Michael Haggerty <mhagger@alum.mit.edu>]

There's one last error formatting niggle below.

On 07/21/2015 08:44 AM, David Turner wrote:
> Add an err argument to log_ref_setup that can explain the reason
> for a failure. This then eliminates the need to manage errno through
> this function since we can just add strerror(errno) to the err string
> when meaningful. No callers relied on errno from this function for
> anything else than the error message.
> 
> Also add err arguments to private functions write_ref_to_lockfile,
> log_ref_write_1, commit_ref_update. This again eliminates the need to
> manage errno in these functions.
> 
> Some error messages are slightly reordered.
> 
> Update of a patch by Ronnie Sahlberg.
> 
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  builtin/checkout.c |   8 ++--
>  refs.c             | 129 ++++++++++++++++++++++++++++++-----------------------
>  refs.h             |   4 +-
>  3 files changed, 81 insertions(+), 60 deletions(-)
> 
> [...]
> diff --git a/refs.c b/refs.c
> index fb568d7..f090720 100644
> --- a/refs.c
> +++ b/refs.c
> [...]
> @@ -3288,12 +3288,17 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
>   * necessary, using the specified lockmsg (which can be NULL).
>   */
>  static int commit_ref_update(struct ref_lock *lock,
> -			     const unsigned char *sha1, const char *logmsg)
> +			     const unsigned char *sha1, const char *logmsg,
> +			     struct strbuf *err)
>  {
>  	clear_loose_ref_cache(&ref_cache);
> -	if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
> +	if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0 ||
>  	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
> -	     log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
> +	     log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0)) {
> +		char *old_msg = strbuf_detach(err, NULL);
> +		strbuf_addf(err, "Cannot update the ref '%s': '%s'",
> +			    lock->ref_name, old_msg);

The above error message has unnecessary quotes around old_msg.

> +		free(old_msg);
>  		unlock_ref(lock);
>  		return -1;
>  	}
> [...]

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu