Re: [PATCH 1/6] refs.c: add an err argument to log_ref_setup

[Junio C Hamano <gitster@pobox.com>]

David Turner <dturner@twopensource.com> writes:

> Add 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.

I think I saw Michael did something similar to other codepaths in
the ref API recently, and going in the same direction is generally a
good thing to do for consistency ;-)

> write_ref_to_lockfile is a private function that calls
> log_ref_setup. Update this function to also take an err argument and
> fill it in.  This again eliminates the need to manage errno in this
> function.

This particular patch is not just about log_ref_setup() and its
private friend write_ref_to_lockfile(), right?  It seems to also
touch commit_ref_update(), log_ref_write(), etc.  The overall
"theme" of this change is to teach parts of the ref API that deal
with writing reflogs to report errors the same way (i.e. as the
remainder of the ref API does by using &err), and that may be a
good single-line summary of the change (aka "Subject").

>  				if (ret) {
> -					fprintf(stderr, _("Can not do reflog for '%s'\n"),
> -					    opts->new_orphan_branch);
> +					fprintf(stderr, _("Can not do reflog for '%s'. %s\n"),
> +					opts->new_orphan_branch, err.buf);

Original is underindented but this makes it even worse.  Push
continuation line further to the right (or restructure the code so
that it does not have to indent too deeply in the first place).

Other than that, this step looks sensible.

Thanks.