Re: [PATCH 02/14] config.c: use kvi for CLI config
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- May 1, 2023, 11:06 UTC
- Message-ID
- <230501.86cz3kjok7.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <a682612cff23449e682023303f17a8be1d73fc31.1682104398.git.gitgitgadget@gmail.com>
On Fri, Apr 21 2023, Glen Choo via GitGitGadget wrote:
Show 9 quoted lines
> @@ -682,17 +677,30 @@ static int config_parse_pair(const char *key, const char *value, > if (git_config_parse_key(key, &canonical_name, NULL)) > return -1; > > - ret = (fn(canonical_name, value, data) < 0) ? -1 : 0; > + ret = (kvi_fn(fn, canonical_name, value, kvi, data) < 0) ? -1 : 0; > free(canonical_name); > return ret; > }
This is pre-existing, but I'd much prefer as we're doing more formalization of this interface if this were just:
ret = kvi_fn(...); ...; return ret;
I.e. a look at the current code shows us that the API users of git_config_parse_parameter() are already doing this coercion themselves, i.e. they only care about "ret < 0".
So let's just hand them the actual return value, rather than doing the needless coercion.
Show 21 quoted lines
> @@ -2423,19 +2429,13 @@ 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) {
> + if (reader->source && reader->source->name) {
> kv_info->filename = strintern(reader->source->name);
> kv_info->linenr = reader->source->linenr;
> kv_info->origin_type = reader->source->origin_type;
> - } 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;
> + kv_info->scope = reader->parsing_scope;
> + } else
> + kvi_from_param(kv_info);Missing a {} here.