Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
> Hi Patrick
>
> On 06/08/2024 10:00, Patrick Steinhardt wrote:
>> We're not releasing the `todo_list` in `sequencer_pick_revisions()` when
>> hitting an error path. Restructure the function to have a common exit
>> path such that we can easily clean up the list and thus plug this memory
>> leak.
>
> This looks good, I've left a couple of small formatting comments below
> if you do end up re-rolling.
Oh, formatting nitpicks, my favourite ;-)
>> @@ -5506,11 +5508,14 @@ int sequencer_pick_revisions(struct repository *r,
>> enum object_type type = oid_object_info(r,
>> &oid,
>> NULL);
Also, if we say
enum object_type type;
type = oid_object_info(r, &oid, NULL);
the result is much easier on the eyes usign the same three lines.
Yes, initializing while declaring may look nicer and in some cases
it may even be necessary, but not this one.
>> - return error(_("%s: can't cherry-pick a %s"),
>> + res = error(_("%s: can't cherry-pick a %s"),
>> name, type_name(type));
>
> This line needs re-indenting to match the changes above.
Thanks.
