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

2016-03-30 Thread Junio C Hamano
Jeff King writes: > On Wed, Mar 30, 2016 at 02:07:14PM -0700, Junio C Hamano wrote: > >> This is a tangent, but am I the only one who finds that the naming >> of functions in config-get API is confusing? Just wondering if we >> should rename the ones that keeps the memory ownership to the config

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

2016-03-30 Thread Jeff King
On Wed, Mar 30, 2016 at 02:07:14PM -0700, Junio C Hamano wrote: > This is a tangent, but am I the only one who finds that the naming > of functions in config-get API is confusing? Just wondering if we > should rename the ones that keeps the memory ownership to the config > subsystem with s/get/pe

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

2016-03-30 Thread Stefan Beller
On Wed, Mar 30, 2016 at 2:07 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >>> diff --git a/builtin/notes.c b/builtin/notes.c >>> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o) >>> static int git_config_get_notes_strategy(const char *key, >>>

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

2016-03-30 Thread Junio C Hamano
Eric Sunshine writes: >> diff --git a/builtin/notes.c b/builtin/notes.c >> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o) >> static int git_config_get_notes_strategy(const char *key, >> enum notes_merge_strategy *strategy) >>

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

2016-03-30 Thread Eric Sunshine
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller wrote: > `value` is just a temporary scratchpad, so we need to make sure it doesn't > leak. It is xstrdup'd in `git_config_get_string` and > `parse_notes_merge_strategy` just compares the string against predefined > values, so no need to keep it aroun

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

2016-03-30 Thread Stefan Beller
`value` is just a temporary scratchpad, so we need to make sure it doesn't leak. It is xstrdup'd in `git_config_get_string` and `parse_notes_merge_strategy` just compares the string against predefined values, so no need to keep it around longer. Make `value` non-const to avoid the cast in the free.