Re: [PATCH 20/29] sequencer: fix leaking string buffer in `commit_staged_changes()`
Hi Patrick
On 04/06/2024 08:19, Patrick Steinhardt wrote:
Show 29 quoted lines
> 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.
>
> PatrickThis and all the other sequencer changes in this series look good to me
Thanks
Phillip
Show 40 quoted lines
> 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;
> - }
> }
> }
>