Subject: Re: [PATCH 09/10] config API: add "string" version of *_value_multi(), fix segfaults

From: Junio C Hamano <[email-masked]>
Date: Fri, 28 Oct 2022 12:16:24 -0700

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

>> Actually, I take it back.  Instead of introducing _string(), how
>> about introducing _bool() and convert those minority callers that do
>> want to see boolean values to use the new one, while rejecting NULLs
>> for everybody else that calls the traditional "get_value" family of
>> functions?  That would "optimize" for the majority of simpler users,
>> wouldn't it?
>
> I don't think the goal should be just to optimize for those current
> users, but to leave the config API in a state where it makes sense
> conceptually.

It is more like guiding a conceptually clean design using the need
of the current users to rein in pursuit of theoretical "elegance".

> Now, if we don't supply the equivalent of the "raw, but multi-value"
> function we'll make it hard to use the API, because now you can't think
> about it as the "multi" just being a list version of what you get with
> the scalar version.

I am not interested in _bool() variant that "stringifies" NULL to
"true" at all.  What I was suggesting was:

 * Reserve the current get and get_multi for those who should have
   been calling config_error_nonbool() themselves (because your
   _string() has not been available to them, they were lazy not to
   bother, leading to NULL dereference given certain end-user data).
   And do the config_error_nonbool() inside the updated get and
   get_multi without introducing _string() variant at all.

 * The above alone WILL break callers who are prepared to handle
   "bool" and "bool plus some other string", because they are fully
   expecting that the get API will give them NULL but the above
   update will instead stop before they see the NULL they are
   prepared to handle themselves.  Introduce _bool variants and make
   them call them.

without any "stringifying" at all.