On Fri, Apr 21 2023, Glen Choo via GitGitGadget wrote:
> @@ -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.
> @@ -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.
