Re: [PATCH] refs.c: repack_without_refs may be called without error string buffer

[Ronnie Sahlberg <sahlberg@google.com>]

On Thu, Nov 20, 2014 at 10:35 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Stefan Beller wrote:
>
>> If we don't pass in the error string buffer, we skip over all
>> parts dealing with preparing error messages.
>
> Please no.
>
> We tried this with the ref transaction code.  When someone wants
> to silence the message, it is cheap enough to do
>
>         struct strbuf ignore = STRBUF_INIT;
>
>         if (thing_that_can_fail_in_an_ignorable_way(..., &ignore)) {
>                 ... handle the failure ...
>         }
>
> The extra lines of code make it obvious that the error message is
> being dropped, which is a very good thing.  The extra work to format a
> message in the error case is not so bad and can be mitigated if the
> error is a common normal case by passing a flag to not consider it an
> error.
>
> Silently losing good diagnostic messages when err == NULL would have
> the opposite effect: when there isn't a spare strbuf to put errors in
> around, it would be tempting for people coding in a hurry to just pass
> NULL, and to readers it would look at first glance like "oh, an
> optional paramter was not passed and we are getting the good default
> behavior".
>
> This is not a theoretical concern --- it actually happened.
>

Fair enough.
Un-LGTM my message above.



> My two cents,
> Jonathan