On 2024-10-01 at 20:31:15, Emily Shaffer wrote: > On Mon, Sep 30, 2024 at 3:04 PM brian m. carlson > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > +const char *git_error_strbuf(struct strbuf *buf, struct git_error err) > > Is the idea that we continue to extend `git_error_strbuf` with more > `git_error_code` values as we add this method of error handling across > the codebase? I'm worried it could get quite unwieldy. Or are you > suggesting that we could write `git_error_strbuf_object_store`, > `git_error_strbuf_hook`, etc to keep the code->string conversion > local? That would let us do custom processing of err.meta depending on > context, too, wouldn't it? My intention was to centralize this processing in one place. Part of the problem we have is that we have many different messages for one error code, so `GIT_ERR_OBJECT_NOT_FOUND` might have "object not found - (HEX)" or a variety of other messages, which makes parsing errors difficult. (As I said, we do this at work, and it's quite annoying.) If folks want to do thing differently, we can, but we should avoid losing the consistent error message behaviour. > > +#define GIT_ERROR_SUCCESS(e) (((e).code == GIT_ERROR_BIT_INIT)) > > +#define GIT_ERROR_SUCCESS_CONSUME(e) ((git_error_free(&(e)).code == GIT_ERROR_BIT_INIT) > > + > > +#define GIT_ERROR_ERRNO(e) (((e).code & GIT_ERROR_BIT_ERRNO) ? ((e).code & 0xffffffff) : -1) > > +#define GIT_ERROR_GITERR(e) (((e).code & GIT_ERROR_BIT_ERRNO) ? -1 : ((e).code & 0xffffffff)) > > Aurgh, too bad we don't get bitfields before C11. :) (Although I am > not sure if that helps with your register-level optimization at that > point... but it probably helps with readability.) > > But, I do wonder if this gluing-together-two-types-into-one-value > thing may break based on endianness? (And if we care? I don't think we > have any tests running on POWER systems, so maybe this falls under the > umbrella of "you should give us tests if you want us to not break > you"?) I don't think this breaks on big-endian systems because we're always treating it as a `uint64_t` and never converting it into bytes. My first laptop was a PowerPC Mac running Linux and I've owned UltraSPARC hardware in the past, so I'm pretty confident in avoiding writing endianness (and alignment) bugs. I wouldn't have written it in a way that broke on less common hardware even if we don't have tests for it. -- brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
