On 7/25/2014 7:28 PM, Matthieu Moy wrote: > Tanay Abhra <tanayabh@xxxxxxxxx> writes: > >> +struct config_set_element { >> + struct hashmap_entry ent; >> + char *key; >> + struct string_list value_list; >> +}; >> + >> +struct configset_list_item { >> + struct config_set_element *e; >> + int value_index; >> +}; > > I originally wondered why you had two levels of pointers, but > config_set_element is not new, just moved. It's OK. > >> +/* >> + * the contents of the list are ordered according to their >> + * position in the config files and order of parsing the files. >> + * (i.e. key-value pair at the last position of .git/config will >> + * be at the last item of the list) >> + */ >> + >> +struct configset_list { > > I wouldn't put a blank line between comment and decl if the comment > applies to the decl. > Noted. >> -int git_config(config_fn_t fn, void *data) >> +static int git_config_raw(config_fn_t fn, void *data) >> { >> return git_config_with_options(fn, data, NULL, 1); >> } > > I would have done this and the new git_config() as a separate patch, but > I do not mind strongly. > >> static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) >> { >> struct config_set_element k; >> @@ -1268,6 +1295,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha >> { >> struct config_set_element *e; >> struct string_list_item *si; >> + struct configset_list_item *l_item; >> struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); >> >> e = configset_find_element(cs, key); >> @@ -1283,6 +1311,13 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha >> hashmap_add(&cs->config_hash, e); >> } >> si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL); >> + >> + if (cs->list.nr + 1 > cs->list.alloc) > > The "if" is already in ALLOC_GROW. > >> + ALLOC_GROW(cs->list.items, cs->list.nr + 20, cs->list.alloc); > > The 20 should be just 1 I guess. You're adding 1 element, and ALLOC_GROW > will take care of allocating more than 1 for you (see alloc_nr and > ALLOC_GROW's defs in cache.h). > Oh, you are are right, I thought alloc grew by one, not by a factor. >> @@ -1318,10 +1356,14 @@ void git_configset_clear(struct config_set *cs) >> hashmap_iter_init(&cs->config_hash, &iter); >> while ((entry = hashmap_iter_next(&iter))) { >> free(entry->key); >> - string_list_clear(&entry->value_list, 0); >> + string_list_clear(&entry->value_list, 1); > > Doesn't this change belong to PATCH 2/6 ? > Yup, you are right again. Thanks. -- 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