Hi,
On Sun, 27 Jul 2008, Christian Couder wrote:
> diff --git a/builtin-merge-base.c b/builtin-merge-base.c
> index 1cb2925..f2c9756 100644
> --- a/builtin-merge-base.c
> +++ b/builtin-merge-base.c
> @@ -38,15 +48,22 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
> usage(merge_base_usage);
> argc--; argv++;
> }
> - if (argc != 3)
> + if (argc < 3)
> usage(merge_base_usage);
> - if (get_sha1(argv[1], rev1key))
> - die("Not a valid object name %s", argv[1]);
> - if (get_sha1(argv[2], rev2key))
> - die("Not a valid object name %s", argv[2]);
> - rev1 = lookup_commit_reference(rev1key);
> - rev2 = lookup_commit_reference(rev2key);
> - if (!rev1 || !rev2)
> +
> + rev1 = get_commit_reference(argv[1]);
> + if (!rev1)
> return 1;
Why do you special case rev1? Is it so special? Just handle it together
with all of the other arguments!
IOW have one commit array, and do not call it "prev".
> - return show_merge_base(rev1, rev2, show_all);
> + argc--; argv++;
> +
> + do {
> + struct commit *rev2 = get_commit_reference(argv[1]);
> + if (!rev2)
> + return 1;
> + ALLOC_GROW(prev2, prev2_nr + 1, prev2_alloc);
> + prev2[prev2_nr++] = rev2;
> + argc--; argv++;
> + } while (argc > 1);
Now, this is ugly. You know beforehand the _exact_ number of arguments,
and yet you dynamically grow the array? Also, why do you use a do { }
while(), when a for () would be much, much clearer?
BTW I seem to recall that get_merge_bases_many() was _not_ the same as
get_merge_octopus(). Could you please remind me what _many() does?
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
