Re: Improved error handling (Was: [PATCH 1/2] sequencer: factor out rewrite_file())

> On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:
>> Yes, I think what you've written here (and below) is quite close to the
>> error_context patches I linked elsewhere in the thread. In other
>> words, I think it's a sane approach.
>
> In contrast to error_context I'd like to keep all exiting
> behavior (die, ignore, etc.) in the hand of the caller and not
> use any callbacks as that makes the control flow much harder to
> follow.
>
>> I agree it might be nice for the error context to have a positive "there
>> was an error" flag. It's probably worth making it redundant with the
>> return code, though, so callers can use whichever style is most
>> convenient for them.
>
> Agreed.
>
> Regarding the API, should it be allowed to pass NULL as error
> pointer to request no additional error handling or should the
> error functions panic on NULL? Allowing NULL makes partial
> conversions possible (e.g. for write_in_full) where old callers
> just pass NULL and check the return values and converted callers
> can use the error struct.
>
> How should translations get handled? Appending ": %s" for
> strerror(errno) might be problematic. Same goes for "outer
> message: inner message" where the helper function just inserts ":
> " between the messages. Is _("%s: %s") (with appropriate
> translator comments) enough to handle these cases?
>
> Suggestions how to name the struct and the corresponding
> functions? My initial idea was struct error and to use error_ as
> prefix, but I'm not sure if struct error is too broad and may
> introduce conflicts with system headers. Also error_ is a little
> long and could be shorted to just err_ but I don't know if that's
> clear enough. The error_ prefix doesn't conflict with many git
> functions, but there are some in usage.c (error_errno, error,
> error_routine).
>
> And as general question, is this approach to error handling
> something we should pursue or are there objections? If there's
> consensus that this might be a good idea I'll look into
> converting some parts of the git code (maybe refs.c) to see how
> it pans out.