Re: [PATCHv4 1/4] notes: don't leak memory in git_config_get_notes_strategy
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 1, 2016, 17:03 UTC
- Message-ID
- <xmqq4mbljm76.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqqh9fljojo.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> 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