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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Neil Horman <nhorman@xxxxxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxx>

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.

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".

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]