Re: [PATCH] config.c: remove unused git_config_key_is_valid()

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 29, 2021, 18:25 UTC
Message-ID: <YVSvix9mY/jOz5K9@nand.local>
In-Reply-To: <patch-1.1-ba40601a511-20210928T125516Z-avarab@gmail.com>

On Tue, Sep 28, 2021 at 02:56:03PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> The git_config_key_is_valid() function got left behind in a
> refactoring in a9bcf6586d1 (alias: use the early config machinery to
> expand aliases, 2017-06-14),
>
> It previously had two users when it was added in 9e9de18f1ad (config:
> silence warnings for command names with invalid keys, 2015-08-24), and
> after 6a1e1bc0a15 (pager: use callbacks instead of configset,
> 2016-09-12) only one remained.
>
> By removing it we can get rid of the "quiet" branches in this
> function, as well as cases where "store_key" is NULL, for which there
> are no other users.

This was the part that I paid most attention to. The only caller which passed "quiet == 1" was git_config_key_is_valid(), which is gone. So all of those "if (!quiet)" conditionals can go away. And the remaining callers all pass a non-null pointer to store_key, so those guard can go away too.

> Out of the 5 callers of git_config_parse_key() only one needs to pass
> a non-NULL "size_t *baselen_", so we could remove the third parameter
> from the public interface. I did not find that potential
> simplification to be worthwhile.

But the guard for baselen being NULL needs to stay, and it does. I agree with you (and Peff) that stopping here is fine.

Thanks, Taylor