Re: [PATCH] refs.c: repack_without_refs may be called without error string buffer
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 20, 2014, 18:35 UTC
- Message-ID
- <20141120183523.GD15945@google.com>
- In-Reply-To
- <1416507040-6576-1-git-send-email-sbeller@google.com>
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.
My two cents, Jonathan