Re: [PATCHv4 1/4] notes: don't leak memory in git_config_get_notes_strategy

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Meh. Rather than reverting the git_config_get_value(), it would have
>> been just as easy and safer (less chance of a future change
>> re-introducing a leak) if you had just inserted the necessary check
>> here:
>>
>>     if (!value)
>>         return  config_error_nonbool(key);
>
> Yup, sounds much more sensible fix that is useful in the longer term
> (and avoids one unnecessary strdup()).

Perhaps like this?

-- >8 --
From: Stefan Beller <sbeller@google.com>
Date: Thu, 31 Mar 2016 11:04:03 -0700
Subject: [PATCH] notes: don't leak memory in git_config_get_notes_strategy

This function asks for the value of a configuration and
after using the value does not have to retain ownership
of the value.  git_config_get_string_const() however is
a function to get a copy of the value, but we forget to
free it before we return.

Because we only need to peek the value without retaining
a pointer to it, use git_config_get_value() to peek into
the value cached in the config API layer.

As git_config_get_value() does not insist the value to be
a string, we'd need to do the "nonbool" check ourselves.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/notes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 52aa9af..c1265eb 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -743,8 +743,10 @@ static int git_config_get_notes_strategy(const char *key,
 {
 	const char *value;
 
-	if (git_config_get_string_const(key, &value))
+	if (git_config_get_value(key, &value))
 		return 1;
+	if (!value)
+		return config_error_nonbool(key);
 	if (parse_notes_merge_strategy(value, strategy))
 		git_die_config(key, "unknown notes merge strategy %s", value);
 
-- 
2.8.0-225-g297c00e