{"thread":{"id":"24229","subject":"[PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each","startedAt":"2010-06-29T08:37:17Z","lastAt":"2010-07-03T06:49:15Z","messageCount":4,"participants":["Alex Riesen","Thiago Farina"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"144451","messageId":"AANLkTimYyHtjCfRtrTgVh3LJeJQeBpdXMRsf3khKatFx@mail.gmail.com","threadId":"24229","inReplyTo":null,"subject":"[PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-06-29T08:37:17Z","receivedAt":"2010-06-29T08:37:17Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The macro is suitable for all these cases and will reduce code of\nneed to just iterate over the items of a string list.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\n> On Tue, Jun 29, 2010 at 10:33, Alex Riesen <raa.lkml@gmail.com> wrote:\n>> BTW, now that I took a look at it... The iteration over string_list\n>> items looks a little overengineered. At least from the point of\n>> view of the existing users of the feature. Wouldn't a simple loop\n>> be just as simple to use (if not simplier) and faster (no uninlineable\n>> function calls and argument preparation and passing needed)?\n>>\n>> #define string_list_foreach(item,list) \\\n>>        for (item = (list)->items; item < (list)->items + (list)->nr; ++item)\n>>\n\nAnd this converts existing callers. Removes more than adds.\n\n notes.c        |   46 ++++++++++++++--------------------------------\n resolve-undo.c |   34 +++++++++++++++-------------------\n 2 files changed, 29 insertions(+), 51 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex 6ee04e7..4d5ad35 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -877,14 +877,6 @@ void string_list_add_refs_from_colon_sep(struct\nstring_list *list,\n \tstrbuf_release(&globbuf);\n }\n\n-static int string_list_add_refs_from_list(struct string_list_item *item,\n-\t\t\t\t\t  void *cb)\n-{\n-\tstruct string_list *list = cb;\n-\tstring_list_add_refs_by_glob(list, item->string);\n-\treturn 0;\n-}\n-\n static int notes_display_config(const char *k, const char *v, void *cb)\n {\n \tint *load_refs = cb;\n@@ -947,30 +939,18 @@ void init_notes(struct notes_tree *t, const char\n*notes_ref,\n \tload_subtree(t, &root_tree, t->root, 0);\n }\n\n-struct load_notes_cb_data {\n-\tint counter;\n-\tstruct notes_tree **trees;\n-};\n-\n-static int load_one_display_note_ref(struct string_list_item *item,\n-\t\t\t\t     void *cb_data)\n-{\n-\tstruct load_notes_cb_data *c = cb_data;\n-\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n-\tinit_notes(t, item->string, combine_notes_ignore, 0);\n-\tc->trees[c->counter++] = t;\n-\treturn 0;\n-}\n-\n struct notes_tree **load_notes_trees(struct string_list *refs)\n {\n+\tstruct string_list_item *item;\n+\tint counter = 0;\n \tstruct notes_tree **trees;\n-\tstruct load_notes_cb_data cb_data;\n \ttrees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *));\n-\tcb_data.counter = 0;\n-\tcb_data.trees = trees;\n-\tfor_each_string_list(load_one_display_note_ref, refs, &cb_data);\n-\ttrees[cb_data.counter] = NULL;\n+\tstring_list_foreach(item, refs) {\n+\t\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n+\t\tinit_notes(t, item->string, combine_notes_ignore, 0);\n+\t\ttrees[counter++] = t;\n+\t}\n+\ttrees[counter] = NULL;\n \treturn trees;\n }\n\n@@ -995,10 +975,12 @@ void init_display_notes(struct display_notes_opt *opt)\n\n \tgit_config(notes_display_config, &load_config_refs);\n\n-\tif (opt && opt->extra_notes_refs)\n-\t\tfor_each_string_list(string_list_add_refs_from_list,\n-\t\t\t\t     opt->extra_notes_refs,\n-\t\t\t\t     &display_notes_refs);\n+\tif (opt && opt->extra_notes_refs) {\n+\t\tstruct string_list_item *item;\n+\t\tstring_list_foreach(item, opt->extra_notes_refs)\n+\t\t\tstring_list_add_refs_by_glob(&display_notes_refs,\n+\t\t\t\t\t\t     item->string);\n+        }\n\n \tdisplay_notes_trees = load_notes_trees(&display_notes_refs);\n \tstring_list_clear(&display_notes_refs, 0);\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 0f50ee0..a3152ff 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -28,29 +28,25 @@ void record_resolve_undo(struct index_state\n*istate, struct cache_entry *ce)\n \tui->mode[stage - 1] = ce->ce_mode;\n }\n\n-static int write_one(struct string_list_item *item, void *cbdata)\n+void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n {\n-\tstruct strbuf *sb = cbdata;\n-\tstruct resolve_undo_info *ui = item->util;\n-\tint i;\n+\tstruct string_list_item *item;\n+\tstring_list_foreach(item, resolve_undo) {\n+\t\tstruct resolve_undo_info *ui = item->util;\n+\t\tint i;\n\n-\tif (!ui)\n-\t\treturn 0;\n-\tstrbuf_addstr(sb, item->string);\n-\tstrbuf_addch(sb, 0);\n-\tfor (i = 0; i < 3; i++)\n-\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n-\tfor (i = 0; i < 3; i++) {\n-\t\tif (!ui->mode[i])\n+\t\tif (!ui)\n \t\t\tcontinue;\n-\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\tstrbuf_addstr(sb, item->string);\n+\t\tstrbuf_addch(sb, 0);\n+\t\tfor (i = 0; i < 3; i++)\n+\t\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n+\t\tfor (i = 0; i < 3; i++) {\n+\t\t\tif (!ui->mode[i])\n+\t\t\t\tcontinue;\n+\t\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\t}\n \t}\n-\treturn 0;\n-}\n-\n-void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n-{\n-\tfor_each_string_list(write_one, resolve_undo, sb);\n }\n\n struct string_list *resolve_undo_read(const char *data, unsigned long size)\n-- \n1.7.1.622.g408a98\n\n\nFrom 44f4d65476df97f2aeb4149f75e3e807437af4a1 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen <raa.lkml@gmail.com>\nDate: Tue, 29 Jun 2010 10:03:41 +0200\nSubject: [PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each\n\nThe macro is suitable for all these cases and will reduce code of\nneed to just iterate over the items of a string list.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n notes.c        |   46 ++++++++++++++--------------------------------\n resolve-undo.c |   34 +++++++++++++++-------------------\n 2 files changed, 29 insertions(+), 51 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex 6ee04e7..4d5ad35 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -877,14 +877,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,\n \tstrbuf_release(&globbuf);\n }\n \n-static int string_list_add_refs_from_list(struct string_list_item *item,\n-\t\t\t\t\t  void *cb)\n-{\n-\tstruct string_list *list = cb;\n-\tstring_list_add_refs_by_glob(list, item->string);\n-\treturn 0;\n-}\n-\n static int notes_display_config(const char *k, const char *v, void *cb)\n {\n \tint *load_refs = cb;\n@@ -947,30 +939,18 @@ void init_notes(struct notes_tree *t, const char *notes_ref,\n \tload_subtree(t, &root_tree, t->root, 0);\n }\n \n-struct load_notes_cb_data {\n-\tint counter;\n-\tstruct notes_tree **trees;\n-};\n-\n-static int load_one_display_note_ref(struct string_list_item *item,\n-\t\t\t\t     void *cb_data)\n-{\n-\tstruct load_notes_cb_data *c = cb_data;\n-\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n-\tinit_notes(t, item->string, combine_notes_ignore, 0);\n-\tc->trees[c->counter++] = t;\n-\treturn 0;\n-}\n-\n struct notes_tree **load_notes_trees(struct string_list *refs)\n {\n+\tstruct string_list_item *item;\n+\tint counter = 0;\n \tstruct notes_tree **trees;\n-\tstruct load_notes_cb_data cb_data;\n \ttrees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *));\n-\tcb_data.counter = 0;\n-\tcb_data.trees = trees;\n-\tfor_each_string_list(load_one_display_note_ref, refs, &cb_data);\n-\ttrees[cb_data.counter] = NULL;\n+\tstring_list_foreach(item, refs) {\n+\t\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n+\t\tinit_notes(t, item->string, combine_notes_ignore, 0);\n+\t\ttrees[counter++] = t;\n+\t}\n+\ttrees[counter] = NULL;\n \treturn trees;\n }\n \n@@ -995,10 +975,12 @@ void init_display_notes(struct display_notes_opt *opt)\n \n \tgit_config(notes_display_config, &load_config_refs);\n \n-\tif (opt && opt->extra_notes_refs)\n-\t\tfor_each_string_list(string_list_add_refs_from_list,\n-\t\t\t\t     opt->extra_notes_refs,\n-\t\t\t\t     &display_notes_refs);\n+\tif (opt && opt->extra_notes_refs) {\n+\t\tstruct string_list_item *item;\n+\t\tstring_list_foreach(item, opt->extra_notes_refs)\n+\t\t\tstring_list_add_refs_by_glob(&display_notes_refs,\n+\t\t\t\t\t\t     item->string);\n+        }\n \n \tdisplay_notes_trees = load_notes_trees(&display_notes_refs);\n \tstring_list_clear(&display_notes_refs, 0);\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 0f50ee0..a3152ff 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -28,29 +28,25 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)\n \tui->mode[stage - 1] = ce->ce_mode;\n }\n \n-static int write_one(struct string_list_item *item, void *cbdata)\n+void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n {\n-\tstruct strbuf *sb = cbdata;\n-\tstruct resolve_undo_info *ui = item->util;\n-\tint i;\n+\tstruct string_list_item *item;\n+\tstring_list_foreach(item, resolve_undo) {\n+\t\tstruct resolve_undo_info *ui = item->util;\n+\t\tint i;\n \n-\tif (!ui)\n-\t\treturn 0;\n-\tstrbuf_addstr(sb, item->string);\n-\tstrbuf_addch(sb, 0);\n-\tfor (i = 0; i < 3; i++)\n-\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n-\tfor (i = 0; i < 3; i++) {\n-\t\tif (!ui->mode[i])\n+\t\tif (!ui)\n \t\t\tcontinue;\n-\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\tstrbuf_addstr(sb, item->string);\n+\t\tstrbuf_addch(sb, 0);\n+\t\tfor (i = 0; i < 3; i++)\n+\t\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n+\t\tfor (i = 0; i < 3; i++) {\n+\t\t\tif (!ui->mode[i])\n+\t\t\t\tcontinue;\n+\t\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\t}\n \t}\n-\treturn 0;\n-}\n-\n-void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n-{\n-\tfor_each_string_list(write_one, resolve_undo, sb);\n }\n \n struct string_list *resolve_undo_read(const char *data, unsigned long size)\n-- \n1.7.1.622.g408a98\n\n"},{"id":"144726","messageId":"20100702205559.GB4941@blimp.localdomain","threadId":"24229","inReplyTo":"AANLkTimYyHtjCfRtrTgVh3LJeJQeBpdXMRsf3khKatFx@mail.gmail.com","subject":"[PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-07-02T20:55:59Z","receivedAt":"2010-07-02T20:55:59Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The macro is suitable for all these cases and will reduce code of\nneed to just iterate over the items of a string list.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nAlex Riesen, Tue, Jun 29, 2010 10:37:17 +0200:\n> > On Tue, Jun 29, 2010 at 10:33, Alex Riesen <raa.lkml@gmail.com> wrote:\n> >> BTW, now that I took a look at it... The iteration over string_list\n> >> items looks a little overengineered. At least from the point of\n> >> view of the existing users of the feature. Wouldn't a simple loop\n> >> be just as simple to use (if not simplier) and faster (no uninlineable\n> >> function calls and argument preparation and passing needed)?\n> >>\n> >> #define string_list_foreach(item,list) \\\n> >>        for (item = (list)->items; item < (list)->items + (list)->nr; ++item)\n> >>\n> \n> And this converts existing callers. Removes more than adds.\n> \n\nRebased on recent Git master (after Julian Philips patches).\n\n notes.c        |   46 ++++++++++++++--------------------------------\n resolve-undo.c |   34 +++++++++++++++-------------------\n 2 files changed, 29 insertions(+), 51 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex 1978244..2d03068 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -877,14 +877,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,\n \tstrbuf_release(&globbuf);\n }\n \n-static int string_list_add_refs_from_list(struct string_list_item *item,\n-\t\t\t\t\t  void *cb)\n-{\n-\tstruct string_list *list = cb;\n-\tstring_list_add_refs_by_glob(list, item->string);\n-\treturn 0;\n-}\n-\n static int notes_display_config(const char *k, const char *v, void *cb)\n {\n \tint *load_refs = cb;\n@@ -947,30 +939,18 @@ void init_notes(struct notes_tree *t, const char *notes_ref,\n \tload_subtree(t, &root_tree, t->root, 0);\n }\n \n-struct load_notes_cb_data {\n-\tint counter;\n-\tstruct notes_tree **trees;\n-};\n-\n-static int load_one_display_note_ref(struct string_list_item *item,\n-\t\t\t\t     void *cb_data)\n-{\n-\tstruct load_notes_cb_data *c = cb_data;\n-\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n-\tinit_notes(t, item->string, combine_notes_ignore, 0);\n-\tc->trees[c->counter++] = t;\n-\treturn 0;\n-}\n-\n struct notes_tree **load_notes_trees(struct string_list *refs)\n {\n+\tstruct string_list_item *item;\n+\tint counter = 0;\n \tstruct notes_tree **trees;\n-\tstruct load_notes_cb_data cb_data;\n \ttrees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *));\n-\tcb_data.counter = 0;\n-\tcb_data.trees = trees;\n-\tfor_each_string_list(refs, load_one_display_note_ref, &cb_data);\n-\ttrees[cb_data.counter] = NULL;\n+\tstring_list_foreach(item, refs) {\n+\t\tstruct notes_tree *t = xcalloc(1, sizeof(struct notes_tree));\n+\t\tinit_notes(t, item->string, combine_notes_ignore, 0);\n+\t\ttrees[counter++] = t;\n+\t}\n+\ttrees[counter] = NULL;\n \treturn trees;\n }\n \n@@ -995,10 +975,12 @@ void init_display_notes(struct display_notes_opt *opt)\n \n \tgit_config(notes_display_config, &load_config_refs);\n \n-\tif (opt && opt->extra_notes_refs)\n-\t\tfor_each_string_list(opt->extra_notes_refs,\n-\t\t\t\t     string_list_add_refs_from_list,\n-\t\t\t\t     &display_notes_refs);\n+\tif (opt && opt->extra_notes_refs) {\n+\t\tstruct string_list_item *item;\n+\t\tstring_list_foreach(item, opt->extra_notes_refs)\n+\t\t\tstring_list_add_refs_by_glob(&display_notes_refs,\n+\t\t\t\t\t\t     item->string);\n+\t}\n \n \tdisplay_notes_trees = load_notes_trees(&display_notes_refs);\n \tstring_list_clear(&display_notes_refs, 0);\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 174ebec..dad5402 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -28,29 +28,25 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)\n \tui->mode[stage - 1] = ce->ce_mode;\n }\n \n-static int write_one(struct string_list_item *item, void *cbdata)\n+void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n {\n-\tstruct strbuf *sb = cbdata;\n-\tstruct resolve_undo_info *ui = item->util;\n-\tint i;\n+\tstruct string_list_item *item;\n+\tstring_list_foreach(item, resolve_undo) {\n+\t\tstruct resolve_undo_info *ui = item->util;\n+\t\tint i;\n \n-\tif (!ui)\n-\t\treturn 0;\n-\tstrbuf_addstr(sb, item->string);\n-\tstrbuf_addch(sb, 0);\n-\tfor (i = 0; i < 3; i++)\n-\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n-\tfor (i = 0; i < 3; i++) {\n-\t\tif (!ui->mode[i])\n+\t\tif (!ui)\n \t\t\tcontinue;\n-\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\tstrbuf_addstr(sb, item->string);\n+\t\tstrbuf_addch(sb, 0);\n+\t\tfor (i = 0; i < 3; i++)\n+\t\t\tstrbuf_addf(sb, \"%o%c\", ui->mode[i], 0);\n+\t\tfor (i = 0; i < 3; i++) {\n+\t\t\tif (!ui->mode[i])\n+\t\t\t\tcontinue;\n+\t\t\tstrbuf_add(sb, ui->sha1[i], 20);\n+\t\t}\n \t}\n-\treturn 0;\n-}\n-\n-void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)\n-{\n-   for_each_string_list(resolve_undo, write_one, sb);\n }\n \n struct string_list *resolve_undo_read(const char *data, unsigned long size)\n-- \n1.7.1.304.g8446\n"},{"id":"144721","messageId":"AANLkTin6vHhkT7Q4h5A1g3OOQYzEPdVIOCWVyzxPacnS@mail.gmail.com","threadId":"24229","inReplyTo":"AANLkTimYyHtjCfRtrTgVh3LJeJQeBpdXMRsf3khKatFx@mail.gmail.com","subject":"Re: [PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-07-02T21:08:51Z","receivedAt":"2010-07-02T21:08:51Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Tue, Jun 29, 2010 at 5:37 AM, Alex Riesen <raa.lkml@gmail.com> wrote:\n> The macro is suitable for all these cases\n\n\"all these cases\" is too vague. Which cases?\n\n> and will reduce code of\n> need to just iterate over the items of a string list.\n>\nA minor comment. There is a typo in the subject:\n\nstring_list_for_each -> string_list_foreach.\n\nAlso, you didn't convert all the cases. There are usages of\nfor_each_string_list under builtin/ directory too (I assume it was\nintentional).\n"},{"id":"144731","messageId":"AANLkTin_a1FUIeFUIs5hR8XRsMYvNtd6xPQi7Zt85sqB@mail.gmail.com","threadId":"24229","inReplyTo":"AANLkTin6vHhkT7Q4h5A1g3OOQYzEPdVIOCWVyzxPacnS@mail.gmail.com","subject":"Re: [PATCH 2/2] Convert the users for of for_each_string_list to string_list_for_each","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2010-07-03T06:49:15Z","receivedAt":"2010-07-03T06:49:15Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Jul 2, 2010 at 23:08, Thiago Farina <tfransosi@gmail.com> wrote:\n> On Tue, Jun 29, 2010 at 5:37 AM, Alex Riesen <raa.lkml@gmail.com> wrote:\n>> The macro is suitable for all these cases\n>\n> \"all these cases\" is too vague. Which cases?\n>\n\nAll the cases of call to for_each_string_list I found and converted.\nBut you're right, I'll improve.\n\n> Also, you didn't convert all the cases. There are usages of\n> for_each_string_list under builtin/ directory too (I assume it was\n> intentional).\n\nEr, no. I just failed to use git grep (used the Vim's builtin).\nI'll check them out, too.\n"}]}