From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 06 May 2011 22:01:06 GMT
Subject: Re: [PATCH v5] Add default merge options for all branches
Message-ID: <7vsjsrxzdp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110506213257.GD20182@elie>

Jonathan Nieder <jrnieder@gmail.com> writes:

>> +static void parse_branch_merge_options(char *bmo)
>> +{
>> +	const char **argv;
>> +	int argc;
>> +	char *buf;
>> +
>> +	if (!bmo)
>> +		return;
>> +	argc = split_cmdline(bmo, &argv);
>> +	if (argc < 0)
>> +		die("Bad branch.%s.mergeoptions string", branch);
>> +	argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
>> +	memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
>
> This is not new code, but it might make sense to do
>
> 	argv[0] = "merge.*.options";
>
> for a saner error message when someone tries
>
> 	[branch "master"]
> 		mergeoptions = --nonsense

Yes, either we stuff a fixed string "branch.*.mergeoptions" to argv[0], or
use another static to recall which variable gave us that value and use
it.  The former is of course easier and less nicer.

>> +	argc++;
>> +	parse_options(argc, argv, NULL, builtin_merge_options,
>> +		      builtin_merge_usage, 0);
>> +	free(buf);
>
> [*]
> This buf seems to be left over.  (I don't think the intent is to
> call free on an uninitialized pointer. ;-))

I also forgot to free argv[].