Re: [PATCH 1/2] sequencer: trivial fix

> Junio C Hamano wrote:
>> Neil Horman <nhorman@tuxdriver.com> writes:
>>
>>> On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote:
>>>> We should free objects before leaving.
>>>>
>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>>> ---
>>>>  sequencer.c | 7 +++++--
>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/sequencer.c b/sequencer.c
>>>> index ab6f8a7..7eeae2f 100644
>>>> --- a/sequencer.c
>>>> +++ b/sequencer.c
>>>> @@ -626,12 +626,15 @@ static int do_pick_commit(struct commit
>>>>  *commit, struct replay_opts *opts)
>>>>  rerere(opts->allow_rerere_auto); } else {
>>>>  int allow = allow_empty(opts, commit);
>>>> - if (allow < 0)
>>>> - return allow;
>>>> + if (allow < 0) {
>>>> + res = allow;
>>>> + goto leave;
>>>> + }
>>>>  if (!opts->no_commit)
>>>>  res = run_git_commit(defmsg, opts, allow);
>>>>  }
>>>>
>>>> +leave:
>>>>  free_message(&msg);
>>>>  free(defmsg);
>>>>
>>>> --
>>>> 1.8.3.rc3.312.g47657de
>>>>
>>>>
>>> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>>
>> This is better done without "goto" in general.
>>
>> The other patch 2/2/ adds one more "we need to exit from the middle
>> of the flow" and makes it look handier to add an exit label here,
>> but it would be even better to express the logic of that patch as a
>> normal cascade of if/else if/..., which is small enough and we do
>> not need the "leave:" label.
>
> Linux kernel developers would disagree. In C 'goto' is quite of then
> the only sane option, and you can see 'goto' used in the Linux kernel
> all over the place for that reason.
>
> In this particular case it also makes perfect sense.
>
>> It probably is better to fold this patch into the other one when it
>> is rerolled to correct the option name gotcha "on the tin".
>
> Why? This patch is standalone and fixes an issue that is independent
> of the other patch. Why squash two patches that do *two* different
> things?
>
> Anyway, I'll happily drop this patch if you want this memory leak to
> remain. But then I'll do the same in the other patch.
>
> This mantra of avodiing 'goto' is not helping anybody.