From: Wincent Colaiuta <win@wincent.com>
Date: Thu, 22 Nov 2007 10:28:17 GMT
Subject: Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive
Message-ID: <825BAB3C-FAB4-4A48-AB94-FAAF5B808CD1@wincent.com>
In-Reply-To: <20071122090828.GC7153@sigill.intra.peff.net>

El 22/11/2007, a las 10:08, Jeff King escribió:

> On Thu, Nov 22, 2007 at 01:02:52AM +0100, Wincent Colaiuta wrote:
>
>> -int interactive_add(const char *path)
>> +int interactive_add(const char **argv, int argc)
>> {
>> -	const char *argv[3] = { "add--interactive", path, NULL };
>> -
>> -	return run_command_v_opt(argv, RUN_GIT_CMD);
>> +	int status;
>> +	const char **args = xmalloc(sizeof(const char *) * (argc + 1));
>> +	args[0] = "add--interactive";
>> +	memcpy((void *)args + sizeof(const char *), argv, sizeof(const  
>> char *) * argc);
>
> The source for the memcpy (argv) is sometimes NULL. The standard  
> forbids
> this, even when the size field is 0. I have no idea if any reasonable
> implementations actually care.

Good point. I've now conditionalized the memcpy with an "if (argc >  
0)". While I was at it I also got rid of the unneeded cast to void *.

> But...
>
>> +		if (argc > 0)
>> +			exit(interactive_add(argv, argc));
>> 		else
>> -			exit(interactive_add(NULL));
>> +			exit(interactive_add(NULL, 0));
>
> There really is no reason to pass NULL at all (since the argc limits  
> us
> anyway), so we can just get rid of the conditional and simplify to:
>
>  exit(interactive_add(argv, argc));
>
> for both cases.

Yes, that's a nice cleanup. Thanks for suggesting it.

Cheers,
Wincent