On Wed, Mar 30, 2016 at 2:07 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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)
>>> {
>>> - const char *value;
>>> + char *value;
>>>
>>> - if (git_config_get_string_const(key, &value))
>>> + if (git_config_get_string(key, &value))
>>> return 1;
>>> if (parse_notes_merge_strategy(value, strategy))
>>> git_die_config(key, "unknown notes merge strategy %s", value);
>>>
>>> + free(value);
>>> return 0;
>>> }
>>
>> Hmm, I thought Peff's suggestion of using git_config_get_value() was
>> accepted as superior since it avoids the allocation altogether, thus
>> no need for free() and no leak.
>
> I agree that this caller can avoid taking ownership of value by
> using git_config_get_value() and that would be a cleaner solution
> here.
>
> 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/peek/ or something.
>
I demonstrated the confusion and disability to read and distinguish
between those names clearly already.
So I'd strongly favor a better naming for functions in config.c
--
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