* [PATCH 1/2] Add a string_list_foreach macro @ 2010-06-29 8:35 Alex Riesen 2010-07-02 20:54 ` Alex Riesen 0 siblings, 1 reply; 5+ messages in thread From: Alex Riesen @ 2010-06-29 8:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: Thiago Farina, git, jrnieder, srabbelier [-- Attachment #1: Type: text/plain, Size: 1467 bytes --] This is more lightweight than for_each_string_list function with callback function and a cookie argument. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> --- On Tue, Jun 29, 2010 at 10:33, Alex Riesen <raa.lkml@gmail.com> wrote: > BTW, now that I took a look at it... The iteration over string_list > items looks a little overengineered. At least from the point of > view of the existing users of the feature. Wouldn't a simple loop > be just as simple to use (if not simplier) and faster (no uninlineable > function calls and argument preparation and passing needed)? > > #define string_list_foreach(item,list) \ > for (item = (list)->items; item < (list)->items + (list)->nr; ++item) > string-list.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/string-list.h b/string-list.h index 63b69c8..188d087 100644 --- a/string-list.h +++ b/string-list.h @@ -24,6 +24,8 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c typedef int (*string_list_each_func_t)(struct string_list_item *, void *); int for_each_string_list(string_list_each_func_t, struct string_list *list, void *cb_data); +#define string_list_foreach(item,list) \ + for (item = (list)->items; item < (list)->items + (list)->nr; ++item) /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); -- 1.7.1.622.g408a98 [-- Attachment #2: 0001-Add-a-string_list_foreach-macro.diff --] [-- Type: application/octet-stream, Size: 1092 bytes --] From b2e7d2dce0cb8a6b50af5ba04ededfb342643a90 Mon Sep 17 00:00:00 2001 From: Alex Riesen <raa.lkml@gmail.com> Date: Tue, 29 Jun 2010 10:02:44 +0200 Subject: [PATCH 1/2] Add a string_list_foreach macro This is more lightweight than for_each_string_list function with callback function and a cookie argument. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> --- string-list.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/string-list.h b/string-list.h index 63b69c8..188d087 100644 --- a/string-list.h +++ b/string-list.h @@ -24,6 +24,8 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c typedef int (*string_list_each_func_t)(struct string_list_item *, void *); int for_each_string_list(string_list_each_func_t, struct string_list *list, void *cb_data); +#define string_list_foreach(item,list) \ + for (item = (list)->items; item < (list)->items + (list)->nr; ++item) /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); -- 1.7.1.622.g408a98 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/2] Add a string_list_foreach macro 2010-06-29 8:35 [PATCH 1/2] Add a string_list_foreach macro Alex Riesen @ 2010-07-02 20:54 ` Alex Riesen 2010-07-03 12:40 ` [PATCH 1/2] Add a for_each_string_list_item macro Alex Riesen 0 siblings, 1 reply; 5+ messages in thread From: Alex Riesen @ 2010-07-02 20:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: Thiago Farina, git, jrnieder, srabbelier, Jay Soffian This is more lightweight than for_each_string_list function with callback function and a cookie argument. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> --- Alex Riesen, Tue, Jun 29, 2010 10:35:15 +0200: > On Tue, Jun 29, 2010 at 10:33, Alex Riesen <raa.lkml@gmail.com> wrote: > > BTW, now that I took a look at it... The iteration over string_list > > items looks a little overengineered. At least from the point of > > view of the existing users of the feature. Wouldn't a simple loop > > be just as simple to use (if not simplier) and faster (no uninlineable > > function calls and argument preparation and passing needed)? > > > > #define string_list_foreach(item,list) \ > > for (item = (list)->items; item < (list)->items + (list)->nr; ++item) > > Rebased on current head (after Julian Philips patches). string-list.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/string-list.h b/string-list.h index 680d600..acf0450 100644 --- a/string-list.h +++ b/string-list.h @@ -24,6 +24,8 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c typedef int (*string_list_each_func_t)(struct string_list_item *, void *); int for_each_string_list(struct string_list *list, string_list_each_func_t, void *cb_data); +#define string_list_foreach(item,list) \ + for (item = (list)->items; item < (list)->items + (list)->nr; ++item) /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); -- 1.7.1.304.g8446 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/2] Add a for_each_string_list_item macro 2010-07-02 20:54 ` Alex Riesen @ 2010-07-03 12:40 ` Alex Riesen 2010-07-03 12:41 ` [PATCH 2/2] Convert the users of for_each_string_list to " Alex Riesen 0 siblings, 1 reply; 5+ messages in thread From: Alex Riesen @ 2010-07-03 12:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Thiago Farina, git, jrnieder, srabbelier, Jay Soffian This is more lightweight than a call to for_each_string_list function with callback function and a cookie argument. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> --- Alex Riesen, Fri, Jul 02, 2010 22:54:17 +0200: > This is more lightweight than for_each_string_list function with > callback function and a cookie argument. > > Signed-off-by: Alex Riesen <raa.lkml@gmail.com> > --- > > Alex Riesen, Tue, Jun 29, 2010 10:35:15 +0200: > > On Tue, Jun 29, 2010 at 10:33, Alex Riesen <raa.lkml@gmail.com> wrote: > > > BTW, now that I took a look at it... The iteration over string_list > > > items looks a little overengineered. At least from the point of > > > view of the existing users of the feature. Wouldn't a simple loop > > > be just as simple to use (if not simplier) and faster (no uninlineable > > > function calls and argument preparation and passing needed)? > > > > > > #define string_list_foreach(item,list) \ > > > for (item = (list)->items; item < (list)->items + (list)->nr; ++item) > > > > > Rebased on current head (after Julian Philips patches). > Changed the macro name to make it look like the for_each* functions. string-list.h | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/string-list.h b/string-list.h index 680d600..a37cae5 100644 --- a/string-list.h +++ b/string-list.h @@ -20,10 +20,12 @@ void string_list_clear(struct string_list *list, int free_util); typedef void (*string_list_clear_func_t)(void *p, const char *str); void string_list_clear_func(struct string_list *list, string_list_clear_func_t clearfunc); -/* Use this function to iterate over each item */ +/* Use this function or the macro below to iterate over each item */ typedef int (*string_list_each_func_t)(struct string_list_item *, void *); int for_each_string_list(struct string_list *list, string_list_each_func_t, void *cb_data); +#define for_each_string_list_item(item,list) \ + for (item = (list)->items; item < (list)->items + (list)->nr; ++item) /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); -- 1.7.1.304.g8446 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Convert the users of for_each_string_list to for_each_string_list_item macro 2010-07-03 12:40 ` [PATCH 1/2] Add a for_each_string_list_item macro Alex Riesen @ 2010-07-03 12:41 ` Alex Riesen 2010-07-06 2:35 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Alex Riesen @ 2010-07-03 12:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Thiago Farina, git, jrnieder, srabbelier, Jay Soffian The rule for selecting the candidates for conversion is: if the callback function returns only 0 (the condition for for_each_string_list to exit early), than it can be safely converted to the macro. A notable exception are the callers in builtin/remote.c. If converted, the readability in the file will suffer greately. Besides, the code is not very performance critical (at the moment, at least): it does output formatting of the list of remotes. Signed-off-by: Alex Riesen <raa.lkml@gmail.com> --- builtin/fetch.c | 42 +++++++++++++----------------------------- builtin/ls-files.c | 45 ++++++++++++++++++++++----------------------- notes.c | 46 ++++++++++++++-------------------------------- resolve-undo.c | 34 +++++++++++++++------------------- 4 files changed, 64 insertions(+), 103 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 6eb1dfe..b0bfaa9 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -544,40 +544,14 @@ static int will_fetch(struct ref **head, const unsigned char *sha1) return 0; } -struct tag_data { - struct ref **head; - struct ref ***tail; -}; - -static int add_to_tail(struct string_list_item *item, void *cb_data) -{ - struct tag_data *data = (struct tag_data *)cb_data; - struct ref *rm = NULL; - - /* We have already decided to ignore this item */ - if (!item->util) - return 0; - - rm = alloc_ref(item->string); - rm->peer_ref = alloc_ref(item->string); - hashcpy(rm->old_sha1, item->util); - - **data->tail = rm; - *data->tail = &rm->next; - - return 0; -} - static void find_non_local_tags(struct transport *transport, struct ref **head, struct ref ***tail) { struct string_list existing_refs = { NULL, 0, 0, 0 }; struct string_list remote_refs = { NULL, 0, 0, 0 }; - struct tag_data data; const struct ref *ref; struct string_list_item *item = NULL; - data.head = head; data.tail = tail; for_each_ref(add_existing, &existing_refs); for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) { @@ -631,10 +605,20 @@ static void find_non_local_tags(struct transport *transport, item->util = NULL; /* - * For all the tags in the remote_refs string list, call - * add_to_tail to add them to the list of refs to be fetched + * For all the tags in the remote_refs string list, + * add them to the list of refs to be fetched */ - for_each_string_list(&remote_refs, add_to_tail, &data); + for_each_string_list_item(item, &remote_refs) { + /* Unless we have already decided to ignore this item... */ + if (item->util) + { + struct ref *rm = alloc_ref(item->string); + rm->peer_ref = alloc_ref(item->string); + hashcpy(rm->old_sha1, item->util); + **tail = rm; + *tail = &rm->next; + } + } string_list_clear(&remote_refs, 0); } diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 1b9b8a8..cf6ab03 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -164,33 +164,32 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce) write_name(ce->name, ce_namelen(ce)); } -static int show_one_ru(struct string_list_item *item, void *cbdata) -{ - const char *path = item->string; - struct resolve_undo_info *ui = item->util; - int i, len; - - len = strlen(path); - if (len < max_prefix_len) - return 0; /* outside of the prefix */ - if (!match_pathspec(pathspec, path, len, max_prefix_len, ps_matched)) - return 0; /* uninterested */ - for (i = 0; i < 3; i++) { - if (!ui->mode[i]) - continue; - printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i], - find_unique_abbrev(ui->sha1[i], abbrev), - i + 1); - write_name(path, len); - } - return 0; -} - static void show_ru_info(void) { + struct string_list_item *item; + if (!the_index.resolve_undo) return; - for_each_string_list(the_index.resolve_undo, show_one_ru, NULL); + + for_each_string_list_item(item, the_index.resolve_undo) { + const char *path = item->string; + struct resolve_undo_info *ui = item->util; + int i, len; + + len = strlen(path); + if (len < max_prefix_len) + continue; /* outside of the prefix */ + if (!match_pathspec(pathspec, path, len, max_prefix_len, ps_matched)) + continue; /* uninterested */ + for (i = 0; i < 3; i++) { + if (!ui->mode[i]) + continue; + printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i], + find_unique_abbrev(ui->sha1[i], abbrev), + i + 1); + write_name(path, len); + } + } } static void show_files(struct dir_struct *dir) diff --git a/notes.c b/notes.c index 1978244..7fd2035 100644 --- a/notes.c +++ b/notes.c @@ -877,14 +877,6 @@ void string_list_add_refs_from_colon_sep(struct string_list *list, strbuf_release(&globbuf); } -static int string_list_add_refs_from_list(struct string_list_item *item, - void *cb) -{ - struct string_list *list = cb; - string_list_add_refs_by_glob(list, item->string); - return 0; -} - static int notes_display_config(const char *k, const char *v, void *cb) { int *load_refs = cb; @@ -947,30 +939,18 @@ void init_notes(struct notes_tree *t, const char *notes_ref, load_subtree(t, &root_tree, t->root, 0); } -struct load_notes_cb_data { - int counter; - struct notes_tree **trees; -}; - -static int load_one_display_note_ref(struct string_list_item *item, - void *cb_data) -{ - struct load_notes_cb_data *c = cb_data; - struct notes_tree *t = xcalloc(1, sizeof(struct notes_tree)); - init_notes(t, item->string, combine_notes_ignore, 0); - c->trees[c->counter++] = t; - return 0; -} - struct notes_tree **load_notes_trees(struct string_list *refs) { + struct string_list_item *item; + int counter = 0; struct notes_tree **trees; - struct load_notes_cb_data cb_data; trees = xmalloc((refs->nr+1) * sizeof(struct notes_tree *)); - cb_data.counter = 0; - cb_data.trees = trees; - for_each_string_list(refs, load_one_display_note_ref, &cb_data); - trees[cb_data.counter] = NULL; + for_each_string_list_item(item, refs) { + struct notes_tree *t = xcalloc(1, sizeof(struct notes_tree)); + init_notes(t, item->string, combine_notes_ignore, 0); + trees[counter++] = t; + } + trees[counter] = NULL; return trees; } @@ -995,10 +975,12 @@ void init_display_notes(struct display_notes_opt *opt) git_config(notes_display_config, &load_config_refs); - if (opt && opt->extra_notes_refs) - for_each_string_list(opt->extra_notes_refs, - string_list_add_refs_from_list, - &display_notes_refs); + if (opt && opt->extra_notes_refs) { + struct string_list_item *item; + for_each_string_list_item(item, opt->extra_notes_refs) + string_list_add_refs_by_glob(&display_notes_refs, + item->string); + } display_notes_trees = load_notes_trees(&display_notes_refs); string_list_clear(&display_notes_refs, 0); diff --git a/resolve-undo.c b/resolve-undo.c index 174ebec..72b4612 100644 --- a/resolve-undo.c +++ b/resolve-undo.c @@ -28,29 +28,25 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce) ui->mode[stage - 1] = ce->ce_mode; } -static int write_one(struct string_list_item *item, void *cbdata) +void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo) { - struct strbuf *sb = cbdata; - struct resolve_undo_info *ui = item->util; - int i; + struct string_list_item *item; + for_each_string_list_item(item, resolve_undo) { + struct resolve_undo_info *ui = item->util; + int i; - if (!ui) - return 0; - strbuf_addstr(sb, item->string); - strbuf_addch(sb, 0); - for (i = 0; i < 3; i++) - strbuf_addf(sb, "%o%c", ui->mode[i], 0); - for (i = 0; i < 3; i++) { - if (!ui->mode[i]) + if (!ui) continue; - strbuf_add(sb, ui->sha1[i], 20); + strbuf_addstr(sb, item->string); + strbuf_addch(sb, 0); + for (i = 0; i < 3; i++) + strbuf_addf(sb, "%o%c", ui->mode[i], 0); + for (i = 0; i < 3; i++) { + if (!ui->mode[i]) + continue; + strbuf_add(sb, ui->sha1[i], 20); + } } - return 0; -} - -void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo) -{ - for_each_string_list(resolve_undo, write_one, sb); } struct string_list *resolve_undo_read(const char *data, unsigned long size) -- 1.7.1.304.g8446 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Convert the users of for_each_string_list to for_each_string_list_item macro 2010-07-03 12:41 ` [PATCH 2/2] Convert the users of for_each_string_list to " Alex Riesen @ 2010-07-06 2:35 ` Junio C Hamano 0 siblings, 0 replies; 5+ messages in thread From: Junio C Hamano @ 2010-07-06 2:35 UTC (permalink / raw) To: Alex Riesen; +Cc: Thiago Farina, git, jrnieder, srabbelier, Jay Soffian Alex Riesen <raa.lkml@gmail.com> writes: > The rule for selecting the candidates for conversion is: if the callback > function returns only 0 (the condition for for_each_string_list to exit > early), than it can be safely converted to the macro. > > A notable exception are the callers in builtin/remote.c. If converted, the > readability in the file will suffer greately. Besides, the code is not very > performance critical (at the moment, at least): it does output formatting of > the list of remotes. > > Signed-off-by: Alex Riesen <raa.lkml@gmail.com> Both patches look very sane. Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-07-06 2:36 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-29 8:35 [PATCH 1/2] Add a string_list_foreach macro Alex Riesen 2010-07-02 20:54 ` Alex Riesen 2010-07-03 12:40 ` [PATCH 1/2] Add a for_each_string_list_item macro Alex Riesen 2010-07-03 12:41 ` [PATCH 2/2] Convert the users of for_each_string_list to " Alex Riesen 2010-07-06 2:35 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).