Jeff King <peff@xxxxxxxx> writes: > And while it may be tempting to say "well, it does not hurt to free them > on the die() path", in my opinion that way madness lies. You may have > access to some local variables that can be freed, but there will be many > other heap allocations that you don't even know about! Here's a toy > example from a similar discussion a few years ago: > > https://lore.kernel.org/git/YNypPeoZTRiOxPPQ@xxxxxxxxxxxxxxxxxxxxxxx/ Yeah, I recall that discussion and the example. Yes, we should not have to crawl up from a direct caller of die() and free everything these stack frames hold. > I.e., this: > > diff --git a/builtin/notes.c b/builtin/notes.c > index cc1163242f..f3d5eda104 100644 > --- a/builtin/notes.c > +++ b/builtin/notes.c > @@ -321,12 +321,8 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset) > die(_("failed to resolve '%s' as a valid ref."), arg); > if (!(value = odb_read_object(the_repository->objects, &object, &type, &len))) > die(_("failed to read object '%s'."), arg); > - if (type != OBJ_BLOB) { > - strbuf_release(&msg->buf); > - free(value); > - free(msg); > + if (type != OBJ_BLOB) > die(_("cannot read note data from non-blob object '%s'."), arg); > - } Much nicer.
