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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 27, 2022, 19:27 UTC
Message-ID: <xmqqsfj9kqwx.fsf@gitster.g>
In-Reply-To: <patch-01.10-eefa253ab1f-20221026T151328Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 14 quoted lines

> 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).

Show 10 quoted lines

>
> 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.