Re: [PATCH v4 1/3] config: rename `git_etc_config()`

[Jeff King <peff@peff.net>]

On Tue, Apr 13, 2021 at 09:11:44AM +0200, Patrick Steinhardt wrote:

> While at it, the function is also refactored to pass memory ownership to
> the caller. This is done to better match semantics of
> `git_global_config()`, which is going to be introduced in the next
> commit.

I think this turned out nicely. There are only two callers, one of which
is already handling freeing the global config. In the other:

> diff --git a/builtin/config.c b/builtin/config.c
> index f71fa39b38..02ed0b3fe7 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  		}
>  	}
>  	else if (use_system_config) {
> -		given_config_source.file = git_etc_gitconfig();
> +		given_config_source.file = git_system_config();
>  		given_config_source.scope = CONFIG_SCOPE_SYSTEM;
>  	} else if (use_local_config) {
>  		given_config_source.file = git_pathdup("config");

We "leak" the result, but I think it is actually an improvement. As you
can see in the context, we are sometimes allocating the field in other
code paths, so this is takes us closer to a world where we can actually
call free(given_config_source.file) to fix the leak in all of the other
code paths. :)

(I don't think any of that needs to be dealt with in this series, of
course).

-Peff