{"thread":{"id":"7876","subject":"[PATCH 0/5] New for_each_revision() helper","startedAt":"2007-04-27T17:00:07Z","lastAt":"2007-04-30T23:19:04Z","messageCount":17,"participants":["Luiz Fernando N. Capitulino","Junio C Hamano","Johannes Schindelin","Alex Riesen","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"40606","messageId":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":null,"subject":"[PATCH 0/5] New for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:07Z","receivedAt":"2007-04-27T17:00:07Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":" Hi,\n\n [This' also a git-send-email test, so, if this fail by showing just\n the first e-mail in the series, do not blame me :)]\n\n This series introduces a helper macro to help programs to walk through\nrevisions (details on the first patch).\n\n Shawn has already alerted me that some people don't like to\n'hide C constructs', but I think that in this case it's useful, as explained\nin the next e-mail.\n\n The complete diff stat is:\n\n builtin-fmt-merge-msg.c | 3 +--\n builtin-log.c | 12 ++++--------\n builtin-shortlog.c | 3 +--\n reachable.c | 3 +--\n revision.h | 11 +++++++++++\n 5 files changed, 18 insertions(+), 14 deletions(-)\n\n But if we subtract the for_each_revision() macro's code we get:\n\n 4 files changed, 7 insertions(+), 14 deletions(-)\n"},{"id":"40610","messageId":"1177693212202-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","subject":"[PATCH 1/5] Introduces for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:08Z","receivedAt":"2007-04-27T17:00:08Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"From: Luiz Fernando N. Capitulino \n\nThis macro may be used to iterate over revisions, so, instead of\ndoing:\n\n\tstruct commit *commit;\n\n\t...\n\n\tprepare_revision_walk(rev);\n\twhile ((commit = get_revision(rev)) != NULL) {\n\n\t...\n\n \t}\n\nNew code should use:\n\n\tstruct commit *commit;\n\n\t...\n\n\tfor_each_revision(commit, rev) {\n\n\t...\n\n\t}\n\nThe only disadvantage is that it's something magical, and the fact that\nit returns a struct commit is not obvious.\n\nOn the other hand it's documented, has the advantage of making the walking\nthrough revisions easier and can save some lines of code.\n\nThis version was suggested by Andy Whitcroft.\n\nSigned-off-by: Luiz Fernando N. Capitulino \n---\n revision.h | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.h b/revision.h\nindex cdf94ad..7be3fc7 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -133,4 +133,15 @@ extern void add_object(struct object *obj,\n extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);\n extern void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode);\n \n+/* helpers */\n+\n+/**\n+ * for_each_revision\t- iterate over revisions\n+ * @commit:\tpointer to a commit object returned for each iteration\n+ * @rev:\trevision pointer\n+ */\n+#define for_each_revision(commit, rev) \\\n+\tfor (prepare_revision_walk(rev); \\\n+\t\t (commit = get_revision(rev)) != NULL; )\n+\n #endif\n-- \n1.5.1.1.372.g4342\n"},{"id":"40611","messageId":"11776932122334-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","subject":"[PATCH 2/5] builtin-fmt-merge-msg.c: Use for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:09Z","receivedAt":"2007-04-27T17:00:09Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"From: Luiz Fernando N. Capitulino \n\nSigned-off-by: Luiz Fernando N. Capitulino \n---\n builtin-fmt-merge-msg.c | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c\nindex 5c145d2..8e1db1c 100644\n--- a/builtin-fmt-merge-msg.c\n+++ b/builtin-fmt-merge-msg.c\n@@ -189,8 +189,7 @@ static void shortlog(const char *name, unsigned char *sha1,\n \tadd_pending_object(rev, branch, name);\n \tadd_pending_object(rev, &head->object, \"^HEAD\");\n \thead->object.flags |= UNINTERESTING;\n-\tprepare_revision_walk(rev);\n-\twhile ((commit = get_revision(rev)) != NULL) {\n+\tfor_each_revision(commit, rev) {\n \t\tchar *oneline, *bol, *eol;\n \n \t\t/* ignore merges */\n-- \n1.5.1.1.372.g4342\n"},{"id":"40609","messageId":"1177693212471-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","subject":"[PATCH 3/5] reachable.c: Use for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:10Z","receivedAt":"2007-04-27T17:00:10Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"From: Luiz Fernando N. Capitulino \n\nSigned-off-by: Luiz Fernando N. Capitulino \n---\n reachable.c | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/reachable.c b/reachable.c\nindex ff3dd34..b69edd8 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -79,7 +79,7 @@ static void walk_commit_list(struct rev_info *revs)\n \tstruct object_array objects = { 0, 0, NULL };\n \n \t/* Walk all commits, process their trees */\n-\twhile ((commit = get_revision(revs)) != NULL)\n+\tfor_each_revision(commit, revs)\n \t\tprocess_tree(commit->tree, &objects, NULL, \"\");\n \n \t/* Then walk all the pending objects, recursively processing them too */\n@@ -195,6 +195,5 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog)\n \t * Set up the revision walk - this will move all commits\n \t * from the pending list to the commit walking list.\n \t */\n-\tprepare_revision_walk(revs);\n \twalk_commit_list(revs);\n }\n-- \n1.5.1.1.372.g4342\n"},{"id":"40608","messageId":"11776932121002-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","subject":"[PATCH 4/5] builtin-shortlog.c: Use for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:11Z","receivedAt":"2007-04-27T17:00:11Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"From: Luiz Fernando N. Capitulino \n\nSigned-off-by: Luiz Fernando N. Capitulino \n---\n builtin-shortlog.c | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex 3f93498..eca802d 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -216,8 +216,7 @@ static void get_from_rev(struct rev_info *rev, struct path_list *list)\n \tchar scratch[1024];\n \tstruct commit *commit;\n \n-\tprepare_revision_walk(rev);\n-\twhile ((commit = get_revision(rev)) != NULL) {\n+\tfor_each_revision(commit, rev) {\n \t\tconst char *author = NULL, *oneline, *buffer;\n \t\tint authorlen = authorlen, onelinelen;\n \n-- \n1.5.1.1.372.g4342\n"},{"id":"40607","messageId":"11776932121665-git-send-email-lcapitulino@mandriva.com.br","threadId":"7876","inReplyTo":"11776932123749-git-send-email-lcapitulino@mandriva.com.br","subject":"[PATCH 5/5] builtin-log.c: Use for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T17:00:12Z","receivedAt":"2007-04-27T17:00:12Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"From: Luiz Fernando N. Capitulino \n\nSigned-off-by: Luiz Fernando N. Capitulino \n---\n builtin-log.c | 12 ++++--------\n 1 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 38bf52f..705050a 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -79,8 +79,7 @@ static int cmd_log_walk(struct rev_info *rev)\n {\n \tstruct commit *commit;\n \n-\tprepare_revision_walk(rev);\n-\twhile ((commit = get_revision(rev)) != NULL) {\n+\tfor_each_revision(commit, rev) {\n \t\tlog_tree_commit(rev, commit);\n \t\tif (!rev->reflog_info) {\n \t\t\t/* we allow cycles in reflog ancestry */\n@@ -390,9 +389,8 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n \to2->flags ^= UNINTERESTING;\n \tadd_pending_object(&check_rev, o1, \"o1\");\n \tadd_pending_object(&check_rev, o2, \"o2\");\n-\tprepare_revision_walk(&check_rev);\n \n-\twhile ((commit = get_revision(&check_rev)) != NULL) {\n+\tfor_each_revision(commit, &check_rev) {\n \t\t/* ignore merges */\n \t\tif (commit->parents && commit->parents->next)\n \t\t\tcontinue;\n@@ -578,8 +576,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (!use_stdout)\n \t\trealstdout = fdopen(dup(1), \"w\");\n \n-\tprepare_revision_walk(&rev);\n-\twhile ((commit = get_revision(&rev)) != NULL) {\n+\tfor_each_revision(commit, &rev) {\n \t\t/* ignore merges */\n \t\tif (commit->parents && commit->parents->next)\n \t\t\tcontinue;\n@@ -716,8 +713,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Unknown commit %s\", limit);\n \n \t/* reverse the list of commits */\n-\tprepare_revision_walk(&revs);\n-\twhile ((commit = get_revision(&revs)) != NULL) {\n+\tfor_each_revision(commit, &revs) {\n \t\t/* ignore merges */\n \t\tif (commit->parents && commit->parents->next)\n \t\t\tcontinue;\n-- \n1.5.1.1.372.g4342\n"},{"id":"40624","messageId":"7vabwtobpg.fsf@assigned-by-dhcp.cox.net","threadId":"7876","inReplyTo":"1177693212202-git-send-email-lcapitulino@mandriva.com.br","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-27T19:32:11Z","receivedAt":"2007-04-27T19:32:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Luiz Fernando N. Capitulino\" \nwrites:\n\n> From: Luiz Fernando N. Capitulino \n>\n> This macro may be used to iterate over revisions, so, instead of\n> doing: ...\n\nI am not a big fan of magic control-flow macros, as it makes the\ncode harder to grok for people new to the codebase.\n"},{"id":"40631","messageId":"20070427181326.14bbbf5c@localhost","threadId":"7876","inReplyTo":"7vabwtobpg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-27T21:13:26Z","receivedAt":"2007-04-27T21:13:26Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"Em Fri, 27 Apr 2007 12:32:11 -0700\nJunio C Hamano escreveu:\n\n| \"Luiz Fernando N. Capitulino\" \n| writes:\n| \n| > From: Luiz Fernando N. Capitulino \n| >\n| > This macro may be used to iterate over revisions, so, instead of\n| > doing: ...\n| \n| I am not a big fan of magic control-flow macros, as it makes the\n| code harder to grok for people new to the codebase.\n\n Yeah, I agree. But I think that any experienced programmer will\nunderstand it.\n\n Anyways, I don't want to raise polemic discussions for minor\nchanges. Feel free to drop this one then.\n\n-- \nLuiz Fernando N. Capitulino\n"},{"id":"40643","messageId":"Pine.LNX.4.64.0704280446180.12006@racer.site","threadId":"7876","inReplyTo":"1177693212202-git-send-email-lcapitulino@mandriva.com.br","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-28T02:46:41Z","receivedAt":"2007-04-28T02:46:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 27 Apr 2007, Luiz Fernando N. Capitulino wrote:\n\n> diff --git a/revision.h b/revision.h\n> index cdf94ad..7be3fc7 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -133,4 +133,15 @@ extern void add_object(struct object *obj,\n> extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);\n> extern void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode);\n> \n> +/* helpers */\n> +\n> +/**\n> + * for_each_revision\t- iterate over revisions\n> + * @commit:\tpointer to a commit object returned for each iteration\n> + * @rev:\trevision pointer\n> + */\n> +#define for_each_revision(commit, rev) \\\n> +\tfor (prepare_revision_walk(rev); \\\n> +\t\t (commit = get_revision(rev)) != NULL; )\n> +\n> #endif\n\nI object to this, additionally to the magic argument that I agree to, on \nthe grounds that it is actually wrong. The first iteration will work on an \n_uninitialized_ \"commit\" variable.\n\nFurthermore, it is not like it was a huge piece of code that is being \nreplaced by a shortcut. There are better places to do some libification \nthan this.\n\nCiao,\nDscho\n"},{"id":"40653","messageId":"20070428115059.GA4888@steel.home","threadId":"7876","inReplyTo":"Pine.LNX.4.64.0704280446180.12006@racer.site","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-28T11:50:59Z","receivedAt":"2007-04-28T11:50:59Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Johannes Schindelin, Sat, Apr 28, 2007 04:46:41 +0200:\n> > +#define for_each_revision(commit, rev) \\\n> > +\tfor (prepare_revision_walk(rev); \\\n> > +\t\t (commit = get_revision(rev)) != NULL; )\n> > +\n> > #endif\n> \n> I object to this, additionally to the magic argument that I agree to, on \n> the grounds that it is actually wrong. The first iteration will work on an \n> _uninitialized_ \"commit\" variable.\n\nNo, it wont. Check it. This code is correct.\n\n> Furthermore, it is not like it was a huge piece of code that is being \n> replaced by a shortcut. There are better places to do some libification \n> than this.\n\nIt is not about libification. It is plain readability issue.\nLook at what list_for_each_* macros did to the source of Linux kernel.\n"},{"id":"40654","messageId":"Pine.LNX.4.64.0704281451510.12006@racer.site","threadId":"7876","inReplyTo":"20070428115059.GA4888@steel.home","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-28T12:52:25Z","receivedAt":"2007-04-28T12:52:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 28 Apr 2007, Alex Riesen wrote:\n\n> Johannes Schindelin, Sat, Apr 28, 2007 04:46:41 +0200:\n> > > +#define for_each_revision(commit, rev) \\\n> > > +\tfor (prepare_revision_walk(rev); \\\n> > > +\t\t (commit = get_revision(rev)) != NULL; )\n> > > +\n> > > #endif\n> > \n> > I object to this, additionally to the magic argument that I agree to, on \n> > the grounds that it is actually wrong. The first iteration will work on an \n> > _uninitialized_ \"commit\" variable.\n> \n> No, it wont. Check it. This code is correct.\n\nYes, sorry, as I admitted in my reply to Junio, there was some serious \nmental temporary disability involved.\n\n> > Furthermore, it is not like it was a huge piece of code that is being \n> > replaced by a shortcut. There are better places to do some \n> > libification than this.\n> \n> It is not about libification. It is plain readability issue. Look at \n> what list_for_each_* macros did to the source of Linux kernel.\n\nPersonally, I find the prepare/get_revision stuff not really too \nunreadable.\n\nCiao,\nDscho\n"},{"id":"40656","messageId":"20070428130201.27630ed4@gnut","threadId":"7876","inReplyTo":"20070428115059.GA4888@steel.home","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-28T16:02:01Z","receivedAt":"2007-04-28T16:02:01Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"Em Sat, 28 Apr 2007 13:50:59 +0200\nAlex Riesen escreveu:\n\n| Johannes Schindelin, Sat, Apr 28, 2007 04:46:41 +0200:\n| \n| > Furthermore, it is not like it was a huge piece of code that is being \n| > replaced by a shortcut. There are better places to do some libification \n| > than this.\n| \n| It is not about libification. It is plain readability issue.\n\n Yes, it's just something I've thought would be worth doing, it's\nnot the libfication work.\n\n| Look at what list_for_each_* macros did to the source of Linux kernel.\n\n BTW, I was considering using Linux kernel's linked list\nimplementation in git, since we have some linked lists around.\n\n But I think people won't like it.\n"},{"id":"40657","messageId":"20070428164836.GA6646@steel.home","threadId":"7876","inReplyTo":"20070428130201.27630ed4@gnut","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-28T16:48:36Z","receivedAt":"2007-04-28T16:48:36Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Luiz Fernando N. Capitulino, Sat, Apr 28, 2007 18:02:01 +0200:\n> > Look at what list_for_each_* macros did to the source of Linux kernel.\n> \n> BTW, I was considering using Linux kernel's linked list\n> implementation in git, since we have some linked lists around.\n\nDo you have some definite place in mind? It's just that the kernel\nlists where intentionally kept very simple, so the list\nimplementations in git probably are as close to the kernel's as it\nsanely possible, at which point bringing them in wont change much.\n\n> But I think people won't like it.\n\nIt depends on what you change and how you do it. Show us\n"},{"id":"40696","messageId":"7vy7kbeke1.fsf@assigned-by-dhcp.cox.net","threadId":"7876","inReplyTo":"20070427181326.14bbbf5c@localhost","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-29T06:59:18Z","receivedAt":"2007-04-29T06:59:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Luiz Fernando N. Capitulino\" \nwrites:\n\n> Em Fri, 27 Apr 2007 12:32:11 -0700\n> Junio C Hamano escreveu:\n>\n> | \"Luiz Fernando N. Capitulino\" \n> | writes:\n> | \n> | > From: Luiz Fernando N. Capitulino \n> | >\n> | > This macro may be used to iterate over revisions, so, instead of\n> | > doing: ...\n> | \n> | I am not a big fan of magic control-flow macros, as it makes the\n> | code harder to grok for people new to the codebase.\n>\n> Yeah, I agree. But I think that any experienced programmer will\n> understand it.\n>\n> Anyways, I don't want to raise polemic discussions for minor\n> changes. Feel free to drop this one then.\n\nI on the other hand like the kernel style list macros.\n\nThe reason I do not like this particular one is because both\noperations you are hiding are not simple operations like\n\"initialize a variable to list head\" or \"follow a single pointer\nin the structure\", but rather heavyweight operations with rather\ncomplex semantics. I would want to make sure that people\nrealize they are calling something heavyweight when they use the\nrevision traversal.\n"},{"id":"40698","messageId":"20070429070616.GV5942@spearce.org","threadId":"7876","inReplyTo":"7vy7kbeke1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-04-29T07:06:17Z","receivedAt":"2007-04-29T07:06:17Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> The reason I do not like this particular one is because both\n> operations you are hiding are not simple operations like\n> \"initialize a variable to list head\" or \"follow a single pointer\n> in the structure\", but rather heavyweight operations with rather\n> complex semantics. I would want to make sure that people\n> realize they are calling something heavyweight when they use the\n> revision traversal.\n\nBut in_merge_base is heavyweight if the two commits are in the\nsame object database, but aren't connected at all. You'll need\nto traverse both histories before aborting and saying there is\nno merge base. That ain't cheap on large trees. But its also a\nsingle line of code.\n\nAnyway, my original problem with this macro was the way it was\ndefined. I think Luiz was able to fix most of my issues with it\nin his latest version, but I still have a personal distaste for\nhiding things like a for(;;) construct in a macro, or allowing a\nmacro parameter to be used more than once within the definition of\nthe macro (unexpected side-effects of evaluating an more than once).\n\n-- \nShawn.\n"},{"id":"40701","messageId":"20070429100433.414bae15@gnut","threadId":"7876","inReplyTo":"20070428164836.GA6646@steel.home","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Luiz Fernando N. Capitulino","fromEmail":"lcapitulino@mandriva.com.br","sentAt":"2007-04-29T13:04:33Z","receivedAt":"2007-04-29T13:04:33Z","isPatch":true,"sender":{"key":"lcapitulino@mandriva.com.br","avatar":null},"body":"Em Sat, 28 Apr 2007 18:48:36 +0200\nAlex Riesen escreveu:\n\n| Luiz Fernando N. Capitulino, Sat, Apr 28, 2007 18:02:01 +0200:\n| > > Look at what list_for_each_* macros did to the source of Linux kernel.\n| > \n| > BTW, I was considering using Linux kernel's linked list\n| > implementation in git, since we have some linked lists around.\n| \n| Do you have some definite place in mind? It's just that the kernel\n| lists where intentionally kept very simple, so the list\n| implementations in git probably are as close to the kernel's as it\n| sanely possible, at which point bringing them in wont change much.\n\n The ones I've looked at, looks like any other linked list I've\nseen in other (not badly written) programs out there.\n\n| > But I think people won't like it.\n| \n| It depends on what you change and how you do it. Show us\n\n Yeah, I want to port some of them to see whether it's\nworth doing.\n\n I've ported the list.h already:\n\nhttp://repo.or.cz/w/git/libgit-gsoc.git?a=commit;h=e389611fc24843d465ef150b361f5b200068e507\n"},{"id":"40769","messageId":"7vps5lbgd3.fsf@assigned-by-dhcp.cox.net","threadId":"7876","inReplyTo":"20070429070616.GV5942@spearce.org","subject":"Re: [PATCH 1/5] Introduces for_each_revision() helper","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-30T23:19:04Z","receivedAt":"2007-04-30T23:19:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> But in_merge_base is heavyweight if the two commits are in the\n> same object database, but aren't connected at all. You'll need\n> to traverse both histories before aborting and saying there is\n> no merge base. That ain't cheap on large trees. But its also a\n> single line of code.\n\nWho said anything about \"a single line of code\"? That is quite\ndifferent from heaviness hidden in a control structure\nlookalike.\n"}]}