Re: [PATCH 20/29] sequencer: fix leaking string buffer in `commit_staged_changes()`

From: Patrick Steinhardt <ps@pks.im>
Date: Jun 4, 2024, 06:45 UTC
Message-ID: <Zl6390B6kUu5TlBM@framework>
In-Reply-To: <758f2321-dc63-4209-8b54-99b74b6bb897@gmail.com>

On Mon, Jun 03, 2024 at 02:14:20PM +0100, Phillip Wood wrote:

Show 17 quoted lines

> Hi Patrick
> 
> On 03/06/2024 10:48, Patrick Steinhardt wrote:
> > @@ -5259,12 +5277,13 @@ static int commit_staged_changes(struct repository *r,
> >   				}
> >   			unuse_commit_buffer:
> >   				repo_unuse_commit_buffer(r, commit, p);
> > -				if (res)
> > -					return res;
> > +				if (res) {
> > +					ret = res;
> > +					goto out;
> > +				}
> 
> Having 'ret' and 'res' in this block is a bit confusing - we could delete
> the declaration for 'res' and  either replace its use with 'ret', or rename
> 'ret' to 'res' in this patch.

Yeah, let's just drop the local `res` variable here and use `ret` instead.

> Apart from that this all looks sensible to me, it is nice to see the number
> of leaks going down.
> 
> Thanks

Thanks!

Patrick