Re: [PATCH v3 07/12] config.c: pass ctx with CLI config

From: Glen Choo <chooglen@google.com>
Date: Jun 23, 2023, 21:41 UTC
Message-ID: <kl6ledm1svoi.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20230623203544.3255750-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 24 quoted lines

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> @@ -2465,16 +2488,10 @@ static int configset_add_value(struct config_reader *reader,
>>  	l_item->e = e;
>>  	l_item->value_index = e->value_list.nr - 1;
>>  
>> -	if (!reader->source)
>> -		BUG("configset_add_value has no source");
>>  	if (reader->source->name) {
>> -		kvi_from_source(reader->source, current_config_scope(), kv_info);
>> +		kvi_from_source(reader->source, kvi_p->scope, kv_info);
>>  	} else {
>> -		/* for values read from `git_config_from_parameters()` */
>> -		kv_info->filename = NULL;
>> -		kv_info->linenr = -1;
>> -		kv_info->origin_type = CONFIG_ORIGIN_CMDLINE;
>> -		kv_info->scope = reader->parsing_scope;
>> +		kvi_from_param(kv_info);
>>  	}
>>  	si->util = kv_info;
>>  
>
> Any reason to remove the "if (!reader->source)" guard? I don't think
> this patch does anything to ensure that reader is present. We can
> probably remove this once "reader" is removed.

Hm you're right, this shouldn't be removed here. I think this a leftover from an ancient version where I dropped references to "reader".