Re: [PATCH 1/3] cache-tree: refactor verification to return error codes

[Patrick Steinhardt <ps@pks.im>]

On Tue, Sep 17, 2024 at 01:05:50PM -0400, Eric Sunshine wrote:
> On Tue, Sep 17, 2024 at 3:13 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/cache-tree.c b/cache-tree.c
> > @@ -890,18 +892,23 @@ static int verify_one(struct repository *r,
> >         struct strbuf tree_buf = STRBUF_INIT;
> >         for (i = 0; i < it->subtree_nr; i++) {
> >                 strbuf_addf(path, "%s/", it->down[i]->name);
> > -               if (verify_one(r, istate, it->down[i]->cache_tree, path))
> > -                       return 1;
> > +               ret = verify_one(r, istate, it->down[i]->cache_tree, path);
> > +               if (ret)
> > +                       goto out;
> 
> Assuming I am understanding correctly that the original code was
> leaking the strbuf by returning early, I was surprised that the commit
> message didn't mention that the patch is also fixing the leak.
> (Probably not worth a reroll, though.)

It just wasn't my main motivation here, so I forgot to mention it. Added
it now though, so it's included in case I'll have to reroll. Thanks!

Patrick