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
