Re: [PATCH 1/3] cache-tree: refactor verification to return error codes
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 18, 2024, 05:11 UTC
- Message-ID
- <Zupg_VUWOfa_i7kU@pks.im>
- In-Reply-To
- <CAPig+cQ472dzChHKgrL_DNRe4KMNHNVxR1Qh3n6M=OgtBkG1+w@mail.gmail.com>
On Tue, Sep 17, 2024 at 01:05:50PM -0400, Eric Sunshine wrote:
Show 18 quoted lines
> 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