{"thread":{"id":"41725","subject":"[PATCH v2] hashmap API: introduce for_each_hashmap_entry() helper macro","startedAt":"2016-03-17T10:38:47Z","lastAt":"2016-03-17T22:39:19Z","messageCount":2,"participants":["Alexander Kuleshov","Karsten Blees"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"281023","messageId":"1458211127-26963-1-git-send-email-kuleshovmail@gmail.com","threadId":"41725","inReplyTo":null,"subject":"[PATCH v2] hashmap API: introduce for_each_hashmap_entry() helper macro","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-03-17T10:38:47Z","receivedAt":"2016-03-17T10:38:47Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"There is common pattern to traverse a hashmap in git source code:\n\n        hashmap_iter_init(map, &iter);\n        while ((entry = hashmap_iter_next(&iter)))\n             // do something with entry\n\nThis patch introduces the for_each_hashmap_entry() macro for more\nsimple and clean usage of this pattern. It encapsulates loop over\na hashmap and makes bypass of a hashmap more readable.\n\nThis patch has not functioal changes, so behaviour still the same\nas before.\n\nSigned-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>\n---\n Documentation/technical/api-hashmap.txt | 5 +++++\n builtin/describe.c                      | 5 +++--\n config.c                                | 7 ++++---\n hashmap.c                               | 8 ++++----\n hashmap.h                               | 4 ++++\n submodule-config.c                      | 3 +--\n test-hashmap.c                          | 4 ++--\n 7 files changed, 23 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt\nindex ad7a5bd..7cb7d2a 100644\n--- a/Documentation/technical/api-hashmap.txt\n+++ b/Documentation/technical/api-hashmap.txt\n@@ -193,6 +193,11 @@ more entries.\n `hashmap_iter_first` is a combination of both (i.e. initializes the iterator\n and returns the first entry, if any).\n \n+`for_each_hashmap_entry`::\n+\n+\tAllows iterate over entries of the given map with the given entry\n+\tand iterator.\n+\n `const char *strintern(const char *string)`::\n `const void *memintern(const void *data, size_t len)`::\n \ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 8a25abe..50e3377 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -274,8 +274,9 @@ static void describe(const char *arg, int last_one)\n \tif (!have_util) {\n \t\tstruct hashmap_iter iter;\n \t\tstruct commit *c;\n-\t\tstruct commit_name *n = hashmap_iter_first(&names, &iter);\n-\t\tfor (; n; n = hashmap_iter_next(&iter)) {\n+\t\tstruct commit_name *n;\n+\n+\t\tfor_each_hashmap_entry(&names, n, &iter) {\n \t\t\tc = lookup_commit_reference_gently(n->peeled, 1);\n \t\t\tif (c)\n \t\t\t\tc->util = n;\ndiff --git a/config.c b/config.c\nindex 7ddb287..392d5a2 100644\n--- a/config.c\n+++ b/config.c\n@@ -1382,16 +1382,17 @@ void git_configset_init(struct config_set *cs)\n \n void git_configset_clear(struct config_set *cs)\n {\n-\tstruct config_set_element *entry;\n \tstruct hashmap_iter iter;\n+\tstruct config_set_element *entry;\n+\n \tif (!cs->hash_initialized)\n \t\treturn;\n \n-\thashmap_iter_init(&cs->config_hash, &iter);\n-\twhile ((entry = hashmap_iter_next(&iter))) {\n+\tfor_each_hashmap_entry(&cs->config_hash, entry, &iter) {\n \t\tfree(entry->key);\n \t\tstring_list_clear(&entry->value_list, 1);\n \t}\n+\n \thashmap_free(&cs->config_hash, 1);\n \tcs->hash_initialized = 0;\n \tfree(cs->list.items);\ndiff --git a/hashmap.c b/hashmap.c\nindex b10b642..c41c12b 100644\n--- a/hashmap.c\n+++ b/hashmap.c\n@@ -141,10 +141,10 @@ void hashmap_free(struct hashmap *map, int free_entries)\n \t\treturn;\n \tif (free_entries) {\n \t\tstruct hashmap_iter iter;\n-\t\tstruct hashmap_entry *e;\n-\t\thashmap_iter_init(map, &iter);\n-\t\twhile ((e = hashmap_iter_next(&iter)))\n-\t\t\tfree(e);\n+\t\tstruct hashmap_entry *entry;\n+\n+\t\tfor_each_hashmap_entry(map, entry, &iter)\n+\t\t\tfree(entry);\n \t}\n \tfree(map->table);\n \tmemset(map, 0, sizeof(*map));\ndiff --git a/hashmap.h b/hashmap.h\nindex ab7958a..772caf2 100644\n--- a/hashmap.h\n+++ b/hashmap.h\n@@ -95,4 +95,8 @@ static inline const char *strintern(const char *string)\n \treturn memintern(string, strlen(string));\n }\n \n+#define for_each_hashmap_entry(map, entry, iter)\t\t\\\n+\tfor (entry = hashmap_iter_first(map, iter); entry;\t\\\n+\t     entry = hashmap_iter_next(iter))\n+\n #endif\ndiff --git a/submodule-config.c b/submodule-config.c\nindex b82d1fb..5a8d7fa 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -73,8 +73,7 @@ static void cache_free(struct submodule_cache *cache)\n \t * allocation of struct submodule entries. Each is allocated by\n \t * their .gitmodule blob sha1 and submodule name.\n \t */\n-\thashmap_iter_init(&cache->for_name, &iter);\n-\twhile ((entry = hashmap_iter_next(&iter)))\n+\tfor_each_hashmap_entry(&cache->for_name, entry, &iter)\n \t\tfree_one_config(entry);\n \n \thashmap_free(&cache->for_path, 1);\ndiff --git a/test-hashmap.c b/test-hashmap.c\nindex cc2891d..917d188 100644\n--- a/test-hashmap.c\n+++ b/test-hashmap.c\n@@ -225,8 +225,8 @@ int main(int argc, char *argv[])\n \t\t} else if (!strcmp(\"iterate\", cmd)) {\n \n \t\t\tstruct hashmap_iter iter;\n-\t\t\thashmap_iter_init(&map, &iter);\n-\t\t\twhile ((entry = hashmap_iter_next(&iter)))\n+\n+\t\t\tfor_each_hashmap_entry(&map, entry, &iter)\n \t\t\t\tprintf(\"%s %s\\n\", entry->key, get_value(entry));\n \n \t\t} else if (!strcmp(\"size\", cmd)) {\n-- \n2.8.0.rc3.212.g1f992f2.dirty\n"},{"id":"281090","messageId":"56EB3217.7090907@gmail.com","threadId":"41725","inReplyTo":"1458211127-26963-1-git-send-email-kuleshovmail@gmail.com","subject":"Re: [PATCH v2] hashmap API: introduce for_each_hashmap_entry() helper macro","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2016-03-17T22:39:19Z","receivedAt":"2016-03-17T22:39:19Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"Am 17.03.2016 um 11:38 schrieb Alexander Kuleshov:\n\n> This patch introduces the for_each_hashmap_entry() macro for more\n\nI'd rather call it 'hashmap_for_each', following the pattern\n'operandtype_operation' used throughout git. E.g. we already have\n'hashmap_get', not 'get_hashmap_entry'.\n\nI realize that existing *for_each* implementations in the git code\nbase are a bit of a mess (except 'sha1_array_for_each_unique'). E.g.\nthere is 'for_each_string_list' and 'for_each_string_list_item'. Both\nloop over the string_list_items of a string_list, but one is named\nafter the collection type, the other after the item type...IMO this\nshouldn't set an example for future code.\n\nThe rest of the patch looks good to me.\n"}]}