On 6/25/2014 1:36 PM, Eric Sunshine wrote: > On Mon, Jun 23, 2014 at 6:41 AM, Tanay Abhra <tanayabh@xxxxxxxxx> wrote: >> Use git_config_get_string instead of git_config to take advantage of >> the config hash-table api which provides a cleaner control flow. >> >> Signed-off-by: Tanay Abhra <tanayabh@xxxxxxxxx> >> --- >> notes.c | 20 ++++++-------------- >> 1 file changed, 6 insertions(+), 14 deletions(-) >> >> diff --git a/notes.c b/notes.c >> index 5fe691d..fc92eec 100644 >> --- a/notes.c >> +++ b/notes.c >> @@ -961,19 +961,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list, >> free(globs_copy); >> } >> >> -static int notes_display_config(const char *k, const char *v, void *cb) >> -{ >> - int *load_refs = cb; >> - >> - if (*load_refs && !strcmp(k, "notes.displayref")) { >> - if (!v) >> - config_error_nonbool(k); >> - string_list_add_refs_by_glob(&display_notes_refs, v); >> - } >> - >> - return 0; >> -} >> - >> const char *default_notes_ref(void) >> { >> const char *notes_ref = NULL; >> @@ -1041,6 +1028,7 @@ struct notes_tree **load_notes_trees(struct string_list *refs) >> void init_display_notes(struct display_notes_opt *opt) >> { >> char *display_ref_env; >> + const char *value; >> int load_config_refs = 0; >> display_notes_refs.strdup_strings = 1; >> >> @@ -1058,7 +1046,11 @@ void init_display_notes(struct display_notes_opt *opt) >> load_config_refs = 1; >> } >> >> - git_config(notes_display_config, &load_config_refs); >> + if (load_config_refs && !git_config_get_string("notes.displayref", &value)) { >> + if (!value) >> + config_error_nonbool("notes.displayref"); >> + string_list_add_refs_by_glob(&display_notes_refs, value); > > Although you correctly diagnose a NULL 'value', you then invoke > string_list_add_refs_by_glob() with that NULL, which will result in a > crash. > > This is not a new error. It dates back to 894a9d33 (Support showing > notes from more than one notes tree; 2010-03-12), but your rewrite > should not retain the brokenness. Whether you fix it in this patch or > a lead-in fix-up patch, the fix deserves mention in the commit > message. Done. Thanks. >> + } >> >> if (opt) { >> struct string_list_item *item; >> -- >> 1.9.0.GIT > -- 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