Re: [PATCH/RFC] builtin: move builtin retrieval to get_builtin()

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

 



On 11/13/2014 07:19 PM, Junio C Hamano wrote:
>>  git.c | 27 +++++++++++++++------------
>>  1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/git.c b/git.c
>> index 18fbf79..e32c5b8 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -487,15 +487,20 @@ static struct cmd_struct commands[] = {
>>  	{ "write-tree", cmd_write_tree, RUN_SETUP },
>>  };
>>  
>> -int is_builtin(const char *s)
>> +struct cmd_struct *get_builtin(const char *s)
> 
> I do not think this has to be extern.
> 
> 	static struct cmd_struct *get_builtin(const char *s)
> 
> perhaps.
> 
>> @@ -519,15 +525,12 @@ static void handle_builtin(int argc, const char **argv)
>>  		argv[0] = cmd = "help";
>>  	}
>>  
>> -	for (i = 0; i < ARRAY_SIZE(commands); i++) {
>> -		struct cmd_struct *p = commands+i;
>> -		if (strcmp(p->cmd, cmd))
>> -			continue;
>> -		if (saved_environment && (p->option & NO_SETUP)) {
>> +	builtin = get_builtin(cmd);
> 
> Nice.
> 
>> +	if (builtin) {
>> +		if (saved_environment && (builtin->option & NO_SETUP))
>>  			restore_env();
>> -			break;
>> -		}
>> -		exit(run_builtin(p, argc, argv));
>> +		else
>> +			exit(run_builtin(builtin, argc, argv));
> 
> This change does not seem to have anything to do with the topic of
> the change.  Why is it necessary?

Does the commit message lack some explanation
or the patch would better be divided into several parts?

I noticed that the patch has been modified (suggested 'static'
scope modification, commit message) and added
to the 'next' branch. So does this mean my task is done
or is there still something I should explain?

Thank you for your corrections.
--
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]