{"thread":{"id":"14711","subject":"[RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","startedAt":"2008-07-28T04:50:23Z","lastAt":"2008-07-30T13:51:00Z","messageCount":7,"participants":["Christian Couder","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"85234","messageId":"20080728065023.743930d6.chriscool@tuxfamily.org","threadId":"14711","inReplyTo":null,"subject":"[RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-07-28T04:50:23Z","receivedAt":"2008-07-28T04:50:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Before this patch \"git merge-base\" accepted only 2 arguments, so\nonly merge bases between 2 references could be computed.\n\nThe purpose of this patch is to make \"git merge-base\" accept more\nthan 2 arguments, so that the merge bases between the first given\nreference and all the other references can be computed.\n\nThis is easily implemented because the \"get_merge_bases_many\"\nfunction in \"commit.c\" already implements the needed computation.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-merge-base.txt |   27 +++++++++++++++--------\n builtin-merge-base.c             |   43 ++++++++++++++++++++++++-------------\n commit.h                         |    2 +\n 3 files changed, 48 insertions(+), 24 deletions(-)\n\n\tI only changed the code (according to what Dscho asked) not the\n\tdocumentation in this version as I had not much time and I need\n\tto think more about it.\n\ndiff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt\nindex 1a7ecbf..463c230 100644\n--- a/Documentation/git-merge-base.txt\n+++ b/Documentation/git-merge-base.txt\n@@ -8,26 +8,35 @@ git-merge-base - Find as good common ancestors as possible for a merge\n \n SYNOPSIS\n --------\n-'git merge-base' [--all] <commit> <commit>\n+'git merge-base' [--all] <commit> <commit>...\n \n DESCRIPTION\n -----------\n \n-'git-merge-base' finds as good a common ancestor as possible between\n-the two commits. That is, given two commits A and B, `git merge-base A\n-B` will output a commit which is reachable from both A and B through\n-the parent relationship.\n+'git-merge-base' finds as good common ancestors as possible between\n+the first commit and the other commits. The default behavior is to\n+output only one as good as possible common ancestor, called a merge\n+base.\n+\n+For example, given two commits A and B, `git merge-base A B` will\n+output a commit which is reachable from both A and B through the\n+parent relationship.\n+\n+Given three commits A, B and C, `git merge-base A B C` will output a\n+commit which is reachable through the parent relationship from both A\n+and B, or from both A and C.\n \n Given a selection of equally good common ancestors it should not be\n relied on to decide in any particular way.\n \n-The 'git-merge-base' algorithm is still in flux - use the source...\n-\n OPTIONS\n -------\n --all::\n-\tOutput all common ancestors for the two commits instead of\n-\tjust one.\n+\tOutput all merge bases for the commits instead of just one. No\n+\tmerge bases in the output should be an ancestor of another\n+\tmerge base in the output. Each common ancestor of the first\n+\tcommit and any of the other commits passed as arguments,\n+\tshould be an ancestor of one of the merge bases in the output.\n \n Author\n ------\ndiff --git a/builtin-merge-base.c b/builtin-merge-base.c\nindex 1cb2925..881363f 100644\n--- a/builtin-merge-base.c\n+++ b/builtin-merge-base.c\n@@ -2,9 +2,11 @@\n #include \"cache.h\"\n #include \"commit.h\"\n \n-static int show_merge_base(struct commit *rev1, struct commit *rev2, int show_all)\n+static int show_merge_base(struct commit **rev, int rev_nr, int show_all)\n {\n-\tstruct commit_list *result = get_merge_bases(rev1, rev2, 0);\n+\tstruct commit_list *result;\n+\n+\tresult = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1, 0);\n \n \tif (!result)\n \t\treturn 1;\n@@ -20,13 +22,21 @@ static int show_merge_base(struct commit *rev1, struct commit *rev2, int show_al\n }\n \n static const char merge_base_usage[] =\n-\"git merge-base [--all] <commit-id> <commit-id>\";\n+\"git merge-base [--all] <commit-id> <commit-id>...\";\n+\n+static struct commit *get_commit_reference(const char *arg)\n+{\n+\tunsigned char revkey[20];\n+\tif (get_sha1(arg, revkey))\n+\t\tdie(\"Not a valid object name %s\", arg);\n+\treturn lookup_commit_reference(revkey);\n+}\n \n int cmd_merge_base(int argc, const char **argv, const char *prefix)\n {\n-\tstruct commit *rev1, *rev2;\n-\tunsigned char rev1key[20], rev2key[20];\n+\tstruct commit **rev;\n \tint show_all = 0;\n+\tint rev_nr = 0;\n \n \tgit_config(git_default_config, NULL);\n \n@@ -38,15 +48,18 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \t\t\tusage(merge_base_usage);\n \t\targc--; argv++;\n \t}\n-\tif (argc != 3)\n+\tif (argc < 3)\n \t\tusage(merge_base_usage);\n-\tif (get_sha1(argv[1], rev1key))\n-\t\tdie(\"Not a valid object name %s\", argv[1]);\n-\tif (get_sha1(argv[2], rev2key))\n-\t\tdie(\"Not a valid object name %s\", argv[2]);\n-\trev1 = lookup_commit_reference(rev1key);\n-\trev2 = lookup_commit_reference(rev2key);\n-\tif (!rev1 || !rev2)\n-\t\treturn 1;\n-\treturn show_merge_base(rev1, rev2, show_all);\n+\n+\trev = xmalloc((argc - 1) * sizeof(*rev));\n+\n+\tdo {\n+\t\tstruct commit *r = get_commit_reference(argv[1]);\n+\t\tif (!r)\n+\t\t\treturn 1;\n+\t\trev[rev_nr++] = r;\n+\t\targc--; argv++;\n+\t} while (argc > 1);\n+\n+\treturn show_merge_base(rev, rev_nr, show_all);\n }\ndiff --git a/commit.h b/commit.h\nindex 77de962..4829a5c 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -121,6 +121,8 @@ int read_graft_file(const char *graft_file);\n struct commit_graft *lookup_commit_graft(const unsigned char *sha1);\n \n extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup);\n+extern struct commit_list *get_merge_bases_many(struct commit *one,\n+\t\tint n, struct commit **twos, int cleanup);\n extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n \n extern int register_shallow(const unsigned char *sha1);\n-- \n1.6.0.rc0.43.g00eb.dirty\n"},{"id":"85238","messageId":"7vabg2wovf.fsf@gitster.siamese.dyndns.org","threadId":"14711","inReplyTo":"20080728065023.743930d6.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-28T05:37:24Z","receivedAt":"2008-07-28T05:37:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> Before this patch \"git merge-base\" accepted only 2 arguments, so\n> only merge bases between 2 references could be computed.\n>\n> The purpose of this patch is to make \"git merge-base\" accept more\n> than 2 arguments, so that the merge bases between the first given\n> reference and all the other references can be computed.\n\nI have trouble with this wording, but I'll comment on the documentation\npart in a separate message.\n\n> +static struct commit *get_commit_reference(const char *arg)\n> +{\n> +\tunsigned char revkey[20];\n> +\tif (get_sha1(arg, revkey))\n> +\t\tdie(\"Not a valid object name %s\", arg);\n> +\treturn lookup_commit_reference(revkey);\n> +}\n\nThis returns a NULL when you feed a tree to the command, and...\n\n>  int cmd_merge_base(int argc, const char **argv, const char *prefix)\n>  {\n> +\tstruct commit **rev;\n>  \tint show_all = 0;\n> +\tint rev_nr = 0;\n>  \n>  \tgit_config(git_default_config, NULL);\n>  \n> @@ -38,15 +48,18 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n>  \t\t\tusage(merge_base_usage);\n>  \t\targc--; argv++;\n>  \t}\n> +\tif (argc < 3)\n>  \t\tusage(merge_base_usage);\n> +\n> +\trev = xmalloc((argc - 1) * sizeof(*rev));\n> +\n> +\tdo {\n> +\t\tstruct commit *r = get_commit_reference(argv[1]);\n> +\t\tif (!r)\n> +\t\t\treturn 1;\n\n... the command silently exits with 1.\n"},{"id":"85241","messageId":"7vzlo2v9vg.fsf@gitster.siamese.dyndns.org","threadId":"14711","inReplyTo":"20080728065023.743930d6.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-28T05:46:43Z","receivedAt":"2008-07-28T05:46:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> Before this patch \"git merge-base\" accepted only 2 arguments, so\n> only merge bases between 2 references could be computed.\n>\n> The purpose of this patch is to make \"git merge-base\" accept more\n> than 2 arguments, so that the merge bases between the first given\n> reference and all the other references can be computed.\n\nAfter thinking about this a bit more, I think that saying \"we now allow\ncomputing merge base between more than 2\" is misleading.\n\nI ended up rewriting almost all the lines.  The points are:\n\n * The command is still about computing merge-bases between two commits,\n   and one of these two commits is the first command line argument as\n   before.  With more than one remaining commits on the command line, you\n   can specify a hypothetical commit that is a merge between all of them.\n\n * We did not formally define what a merge-base is.  Define it.\n\n * With a proper definition of merge-base as \"best common ancestors\", with\n   the definition of \"good/better/best\" between common ancestors, we do\n   not have to say anything redundant about --all.  It outputs all merge\n   bases, as opposed to one chosen at random.\n\n * Have definition and give illustrations in a new Discussion section.\n\n---\n Documentation/git-merge-base.txt |   77 ++++++++++++++++++++++++++++++++-----\n 1 files changed, 66 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt\nindex 1a7ecbf..fd4b5c9 100644\n--- a/Documentation/git-merge-base.txt\n+++ b/Documentation/git-merge-base.txt\n@@ -8,26 +8,81 @@ git-merge-base - Find as good common ancestors as possible for a merge\n \n SYNOPSIS\n --------\n-'git merge-base' [--all] <commit> <commit>\n+'git merge-base' [--all] <commit> <commit>...\n \n DESCRIPTION\n -----------\n \n-'git-merge-base' finds as good a common ancestor as possible between\n-the two commits. That is, given two commits A and B, `git merge-base A\n-B` will output a commit which is reachable from both A and B through\n-the parent relationship.\n+'git-merge-base' finds best common ancestor(s) between two commits to use\n+in a three-way merge.  One common ancestor is 'better' than another common\n+ancestor if the latter is an ancestor of the former.  A common ancestor\n+that does not have any better common ancestor than it is a 'best common\n+ancestor', i.e. a 'merge base'.  Note that there can be more than one\n+merge bases between two commits.\n \n-Given a selection of equally good common ancestors it should not be\n-relied on to decide in any particular way.\n-\n-The 'git-merge-base' algorithm is still in flux - use the source...\n+Among the two commits to compute their merge bases, one is specified by\n+the first commit argument on the command line; the other commit is a\n+(possibly hypothetical) commit that is a merge across all the remaining\n+commits on the command line.  As the most common special case, giving only\n+two commits from the command line means computing the merge base between\n+the given two commits.\n \n OPTIONS\n -------\n --all::\n-\tOutput all common ancestors for the two commits instead of\n-\tjust one.\n+\tOutput all merge bases for the commits, instead of just one.\n+\n+DISCUSSION\n+----------\n+\n+Given two commits 'A' and 'B', `git merge-base A B` will output a commit\n+which is reachable from both 'A' and 'B' through the parent relationship.\n+\n+For example, with this topology:\n+\n+                 o---o---o---B\n+                /\n+        ---o---1---o---o---o---A\n+\n+the merge base between 'A' and 'B' is '1'.\n+\n+Given three commits 'A', 'B' and 'C', `git merge-base A B C` will compute the\n+merge base between 'A' and an hypothetical commit 'M', which is a merge\n+between 'B' and 'C'.  For example, with this topology:\n+\n+               o---o---o---o---C\n+              /\n+             /   o---o---o---B\n+            /   /\n+        ---2---1---o---o---o---A\n+\n+the result of `git merge-base A B C` is '1'.  This is because the\n+equivalent topology with a merge commit 'M' between 'B' and 'C' is:\n+\n+\n+               o---o---o---o---o\n+              /                 \\\n+             /   o---o---o---o---M\n+            /   /\n+        ---2---1---o---o---o---A\n+\n+and the result of `git merge-base A M` is '1'.  Commit '2' is also a\n+common ancestor between 'A' and 'M', but '1' is a better common ancestor,\n+because '2' is an ancestor of '1'.  Hence, '2' is not a merge base.\n+\n+When the history involves criss-cross merges, there can be more than one\n+'best' common ancestors between two commits.  For example, with this\n+topology:\n+\n+       ---1---o---A\n+\t   \\ /\n+\t    X\n+\t   / \\\n+       ---2---o---o---B\n+\n+both '1' and '2' are merge-base of A and B.  Neither one is better than\n+the other (both are 'best' merge base).  When `--all` option is not given,\n+it is unspecified which best one is output.\n \n Author\n ------\n"},{"id":"85277","messageId":"alpine.DEB.1.00.0807281328520.2725@eeepc-johanness","threadId":"14711","inReplyTo":"20080728065023.743930d6.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-28T11:33:45Z","receivedAt":"2008-07-28T11:33:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 28 Jul 2008, Christian Couder wrote:\n\n> +\trev = xmalloc((argc - 1) * sizeof(*rev));\n> +\n> +\tdo {\n> +\t\tstruct commit *r = get_commit_reference(argv[1]);\n> +\t\tif (!r)\n> +\t\t\treturn 1;\n> +\t\trev[rev_nr++] = r;\n> +\t\targc--; argv++;\n> +\t} while (argc > 1);\n> +\n> +\treturn show_merge_base(rev, rev_nr, show_all);\n\n\trev = xmalloc((argc - 1) * sizeof(*rev));\n\n\tfor (rev_nr = 0; rev_nr + 1 < argc; rev_nr++) {\n\t\trev[rev_nr] = get_commit_reference(argv[rev_nr + 1]);\n\t\tif (!rev[rev_nr])\n\t\t\treturn !!error(\"Does not refer to a commit: '%s'\",\n\t\t\t\targv[rev_nr + 1]);\n\t}\n\n\treturn show_merge_base(rev, rev_nr, show_all);\n\nI do not know about you, but I think this is not only shorter (in spite of \nadding a helpful error message), but also simpler to understand (not using \nconvoluted do { } while logic), and therefore superior.\n\nYour performance argument is weak IMHO, as this is not a big performance \nhit, and command line parameter parsing is definitely not performance \ncritical.\n\nCiao,\nDscho\n"},{"id":"85404","messageId":"200807290724.16157.chriscool@tuxfamily.org","threadId":"14711","inReplyTo":"7vabg2wovf.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-07-29T05:24:15Z","receivedAt":"2008-07-29T05:24:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 28 juillet 2008, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n>\n> > +static struct commit *get_commit_reference(const char *arg)\n> > +{\n> > +\tunsigned char revkey[20];\n> > +\tif (get_sha1(arg, revkey))\n> > +\t\tdie(\"Not a valid object name %s\", arg);\n> > +\treturn lookup_commit_reference(revkey);\n> > +}\n>\n> This returns a NULL when you feed a tree to the command, and...\n>\n> >  int cmd_merge_base(int argc, const char **argv, const char *prefix)\n> >  {\n> > +\tstruct commit **rev;\n> >  \tint show_all = 0;\n> > +\tint rev_nr = 0;\n> >\n> >  \tgit_config(git_default_config, NULL);\n> >\n> > @@ -38,15 +48,18 @@ int cmd_merge_base(int argc, const char **argv,\n> > const char *prefix) usage(merge_base_usage);\n> >  \t\targc--; argv++;\n> >  \t}\n> > +\tif (argc < 3)\n> >  \t\tusage(merge_base_usage);\n> > +\n> > +\trev = xmalloc((argc - 1) * sizeof(*rev));\n> > +\n> > +\tdo {\n> > +\t\tstruct commit *r = get_commit_reference(argv[1]);\n> > +\t\tif (!r)\n> > +\t\t\treturn 1;\n>\n> ... the command silently exits with 1.\n\nIn \"master\" there is:\n\n\trev1 = lookup_commit_reference(rev1key);\n\trev2 = lookup_commit_reference(rev2key);\n\tif (!rev1 || !rev2)\n\t\treturn 1;\n\treturn show_merge_base(rev1, rev2, show_all);\n\nso I think you found a bug in the current code.\nI will post a patch to fix it soon.\n\nIt will \"die\" (with an error ùmessage) in case \"lookup_commit_reference\" \nreturns NULL. I hope it's ok.\n\nThanks,\nChristian.\n"},{"id":"85569","messageId":"200807300652.41404.chriscool@tuxfamily.org","threadId":"14711","inReplyTo":"alpine.DEB.1.00.0807281328520.2725@eeepc-johanness","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2008-07-30T04:52:41Z","receivedAt":"2008-07-30T04:52:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nLe lundi 28 juillet 2008, Johannes Schindelin a écrit :\n> Hi,\n>\n> On Mon, 28 Jul 2008, Christian Couder wrote:\n> > +\trev = xmalloc((argc - 1) * sizeof(*rev));\n> > +\n> > +\tdo {\n> > +\t\tstruct commit *r = get_commit_reference(argv[1]);\n> > +\t\tif (!r)\n> > +\t\t\treturn 1;\n> > +\t\trev[rev_nr++] = r;\n> > +\t\targc--; argv++;\n> > +\t} while (argc > 1);\n> > +\n> > +\treturn show_merge_base(rev, rev_nr, show_all);\n>\n> \trev = xmalloc((argc - 1) * sizeof(*rev));\n>\n> \tfor (rev_nr = 0; rev_nr + 1 < argc; rev_nr++) {\n> \t\trev[rev_nr] = get_commit_reference(argv[rev_nr + 1]);\n> \t\tif (!rev[rev_nr])\n> \t\t\treturn !!error(\"Does not refer to a commit: '%s'\",\n> \t\t\t\targv[rev_nr + 1]);\n> \t}\n>\n> \treturn show_merge_base(rev, rev_nr, show_all);\n>\n> I do not know about you, but I think this is not only shorter (in spite\n> of adding a helpful error message), but also simpler to understand (not\n> using convoluted do { } while logic), and therefore superior.\n\nIn my last version the loop is reduced to:\n\n+\tdo {\n+\t\trev[rev_nr++] = get_commit_reference(argv[1]);\n+\t\targc--; argv++;\n+\t} while (argc > 1);\n\nso it's very simple.\n\nAnd the stop condition is simpler in my version.\n\n> Your performance argument is weak IMHO, as this is not a big performance\n> hit, and command line parameter parsing is definitely not performance\n> critical.\n\nIt feels a bit sloppy though.\n\nRegards,\nChristian.\n"},{"id":"85619","messageId":"alpine.LSU.1.00.0807301546360.3486@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"14711","inReplyTo":"200807300652.41404.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH v3] merge-base: teach \"git merge-base\" to accept more than 2 arguments","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-30T13:51:00Z","receivedAt":"2008-07-30T13:51:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 30 Jul 2008, Christian Couder wrote:\n\n> Le lundi 28 juillet 2008, Johannes Schindelin a écrit :\n>\n> > On Mon, 28 Jul 2008, Christian Couder wrote:\n> > > +\trev = xmalloc((argc - 1) * sizeof(*rev));\n> > > +\n> > > +\tdo {\n> > > +\t\tstruct commit *r = get_commit_reference(argv[1]);\n> > > +\t\tif (!r)\n> > > +\t\t\treturn 1;\n> > > +\t\trev[rev_nr++] = r;\n> > > +\t\targc--; argv++;\n> > > +\t} while (argc > 1);\n> > > +\n> > > +\treturn show_merge_base(rev, rev_nr, show_all);\n> >\n> > \trev = xmalloc((argc - 1) * sizeof(*rev));\n> >\n> > \tfor (rev_nr = 0; rev_nr + 1 < argc; rev_nr++) {\n> > \t\trev[rev_nr] = get_commit_reference(argv[rev_nr + 1]);\n> > \t\tif (!rev[rev_nr])\n> > \t\t\treturn !!error(\"Does not refer to a commit: '%s'\",\n> > \t\t\t\targv[rev_nr + 1]);\n> > \t}\n> >\n> > \treturn show_merge_base(rev, rev_nr, show_all);\n> >\n> > I do not know about you, but I think this is not only shorter (in spite\n> > of adding a helpful error message), but also simpler to understand (not\n> > using convoluted do { } while logic), and therefore superior.\n> \n> In my last version the loop is reduced to:\n> \n> +\tdo {\n> +\t\trev[rev_nr++] = get_commit_reference(argv[1]);\n> +\t\targc--; argv++;\n> +\t} while (argc > 1);\n> \n> so it's very simple.\n> \n> And the stop condition is simpler in my version.\n\nDoes not matter.  The logic is convoluted, i.e. not how (most) humans \nthink.  And you cheat by putting the argc and argv statements in one line.\n\nAnd you do not use argc and argv after the loop, so those statements are \npointless.\n\nAnd you cheat by not needing that die() anymore because you posted a \n(good!) patch _after_ I sent my version.\n\nSo it could e as short as this:\n\n \twhile (++rev_nr < argc)\n \t\trev[rev_nr - 1] = get_commit_reference(argv[rev_nr]);\n\nI would contend that the for() version is more readable.\n\n> > Your performance argument is weak IMHO, as this is not a big \n> > performance hit, and command line parameter parsing is definitely not \n> > performance critical.\n> \n> It feels a bit sloppy though.\n\nSure.\n\nIf you think that obviousness in a command line parsing is less important \nthan performance.\n\nCiao,\nDscho\n"}]}