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

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 4 Jun 2024 09:19:05 +0200

On Tue, Jun 04, 2024 at 08:45:11AM +0200, Patrick Steinhardt wrote:
> On Mon, Jun 03, 2024 at 02:14:20PM +0100, Phillip Wood wrote:
> > 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.

For the record, below is the diff to address this. I'll refrain from
sending out this whole patch series again for now to only address this
one issue, but will include it if there are more things that need to be
handled.

Patrick

diff --git a/sequencer.c b/sequencer.c
index 9e90084692..cc57a30883 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5253,7 +5253,6 @@ static int commit_staged_changes(struct repository *r,
 				 * We need to update the squash message to skip
 				 * the latest commit message.
 				 */
-				int res = 0;
 				struct commit *commit;
 				const char *msg;
 				const char *path = rebase_path_squash_msg();
@@ -5266,22 +5265,22 @@ static int commit_staged_changes(struct repository *r,
 
 				p = repo_logmsg_reencode(r, commit, NULL, encoding);
 				if (!p)  {
-					res = error(_("could not parse commit %s"),
+					ret = error(_("could not parse commit %s"),
 						    oid_to_hex(&commit->object.oid));
 					goto unuse_commit_buffer;
 				}
 				find_commit_subject(p, &msg);
 				if (write_message(msg, strlen(msg), path, 0)) {
-					res = error(_("could not write file: "
+					ret = error(_("could not write file: "
 						       "'%s'"), path);
 					goto unuse_commit_buffer;
 				}
+
+				ret = 0;
 			unuse_commit_buffer:
 				repo_unuse_commit_buffer(r, commit, p);
-				if (res) {
-					ret = res;
+				if (ret)
 					goto out;
-				}
 			}
 		}