Junio C Hamano <gitster@xxxxxxxxx> writes:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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@xxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html