Re: [PATCH 4/6] Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option

From: Jens Lehmann <jens.lehmann@web.de>
Date: Feb 24, 2011, 20:44 UTC
Message-ID: <4D66C32E.5090103@web.de>
In-Reply-To: <7vei6y5ol6.fsf@alter.siamese.dyndns.org>

Am 24.02.2011 00:16, schrieb Junio C Hamano:

Show 14 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> diff --git a/submodule.c b/submodule.c
>> index cccd728..b477c3c 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -113,7 +113,7 @@ int parse_submodule_config_option(const char *var, const char *value)
>>  		if (!config)
>>  			config = string_list_append(&config_fetch_recurse_submodules_for_name,
>>  						    strbuf_detach(&submodname, NULL));
>> -		config->util = git_config_bool(var, value) ? (void *)1 : NULL;
>> +		config->util = (void *)(size_t)parse_fetch_recurse_submodules_arg(value);
> 
> What is this double-cast about?

64bit gcc warns when I drop either of them because a 32bit integer is assigned to a 64bit wide pointer here.

Show 8 quoted lines

>> @@ -376,8 +376,13 @@ int fetch_populated_submodules(int num_options, const char **options,
>> ...
>> +				if ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_OFF)
>>  					continue;
>> +				if ((size_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_ON_DEMAND) {
> 
> Likewise here; size_t feels a strange type to cast to in this comparison
> between (void *) and an enum, no?

I get a warning here if I drop the second cast. gcc doesn't warn if I drop the first one, but that is just because the enum value happens to be 0 there. So I added that cast there too to be on the safe side in case the value changes in the future and to be consistent to other readers of this code.