Subject: Re: [PATCH 01/10] config API: have *_multi() return an "int" and take a "dest"

From: Junio C Hamano <[email-masked]>
Date: Thu, 27 Oct 2022 12:27:58 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> By changing the *_multi() function to return an "int" for the status
> and to write to a "const struct string_list **dest" parameter we can
> avoid losing this information. API callers can now do:
>
> 	const struct string_list *dest;
> 	int ret;
>
> 	ret = git_config_get_value_multi(key, &dest);
> 	if (ret < 1)
> 		die("bad key: %s", key);
> 	else if (ret)
> 		; /* key does not exist */
> 	else
> 		; /* got key, can use "dest" */

It is a good thing to allow the callers to tell "no such key-value
pair exists", "key is malformed", and "here are the values for the
key".  And the above if/else if/else cascade is a reasonable
interface to give the callers for that (modulo that "negative is
bad" should be kept to match our API convention).

>
> A "get_knownkey_value_multi" variant is also provided, which will
> BUG() out in the "ret < 1" case. This is useful in the cases where we
> hardcode the keyname in our source code, and therefore use the more
> idiomatic pattern of:
>
> 	if (!git_config_get_value_multi(key, &dest)
> 		; /* got key, can use "dest" */
> 	else
> 		; /* key does not exist */

I doubt it is a good idea to add such a specialized interface begin
with.  Let's not bloat the API for little benefit.