On Wed, Feb 22, 2023 at 11:34 AM Jeff King <peff@xxxxxxxx> wrote: > > On Fri, Feb 17, 2023 at 02:49:51PM -0800, Emily Shaffer wrote: > > > > Personally, I'd like to see some sort of standard error type (whether > > > integral or not) that would let us do more bubbling up of errors and > > > less die(). I don't know if that's in the cards, but I thought I'd > > > suggest it in case other folks are interested. > > > > Yes!!! We have talked about this a lot internally - but this is one > > thing that will be difficult to introduce into Git without making > > parts of the codebase a little uglier. Since obviously C doesn't have > > an intrinsic to do this, we'll have to roll our own, which means that > > manipulating it consistently at function exits might end up pretty > > ugly. So hearing that there's interest outside of my team to come up > > with such a type makes me optimistic that we can figure out a > > neat-enough solution. > > Here are some past discussions on what I thought would be a good > approach to error handling. The basic idea is to replace the "pass a > strbuf that people shove error messages into" approach with an error > context struct that has a callback. And that callback can then stuff > them into a strbuf, or report them directly, or even die. Thanks! I'll give these a read in detail soon, I appreciate you digging them up. > > This thread sketches out the idea, though sadly I no longer have the > more fleshed-out patches I mentioned there: > > https://lore.kernel.org/git/20160927191955.mympqgylrxhkp24n@xxxxxxxxxxxxxxxxxxxxx/ > > And then the sub-thread starting here discusses a similar approach: > > https://lore.kernel.org/git/20171103191309.sth4zjokgcupvk2e@xxxxxxxxxxxxxxxxxxxxx/ > > It does mean passing a "struct error_context" just about everywhere. > Though since the context doesn't change very much and most calls are > just forwarding it along, it would probably also be reasonable to have a > thread-local global context, and push/pop from it (sort of a poor man's > dynamic scoping). > > One thing that strategy doesn't help with, though, that your > libification might want: it's not very machine-readable. The error > reporters would still fundamentally be working with strings. So a > libified process can know "OK, writing this ref failed, and I have some > error messages in a buffer". But the calling code can't know specifics > like "it failed because we tried to open file 'foo' and it got EPERM". > We _could_ design an error context that stores individual errno values > or codes in a list, but having each caller report those specific errors > is a much bigger job (and ongoing maintenance burden as we catalogue and > give an identifier to each error). Is there a reason not to use this kind of struct and provide library-specific error code enums, though, I wonder? You're right that parsing the error string is really bad for the caller, for anything besides just logging it. But it seems somewhat reasonable to expect that any call from config library returning an integer error code is referring to enum config_errors... > > -Peff
