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. -- 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