[Added Florian to Cc] Elijah Newren wrote: > Commits 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax", > 2008-07-26) and 3dd4e7320d ("Teach rev-parse the ... syntax.", 2006-07-04) > taught rev-parse new syntax, and used lookup_commit_reference() as part of > their logic. Neither usage checked the returned commit to see if it was > non-NULL before using it. Check for NULL and ensure an appropriate error > is reported to the user. > > Reported by Florian Weimer and Todd Zullinger. > > Helped-by: Jeff King <peff@xxxxxxxx> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > --- The output is now much more consistent with other invalid input. The only (minor) difference I noticed was when using the fff...fff form. With exactly 40 chars, rev-parse prints both refs separately and then the full input string before the "fatal:" error. I doubt it's terribly important. # exactly 40 chars $ ./git-rev-parse ffffffffffffffffffffffffffffffffffffffff...ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffff...ffffffffffffffffffffffffffffffffffffffff fatal: ambiguous argument 'ffffffffffffffffffffffffffffffffffffffff...ffffffffffffffffffffffffffffffffffffffff': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' # not 40 chars $ ./git-rev-parse fffffffffffffffffffffffffffffffffffffff...fffffffffffffffffffffffffffffffffffffff fffffffffffffffffffffffffffffffffffffff...fffffffffffffffffffffffffffffffffffffff fatal: ambiguous argument 'fffffffffffffffffffffffffffffffffffffff...fffffffffffffffffffffffffffffffffffffff': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' > I would have used a Reported-by tag for Florian and Todd, but looking at > the bugzilla.redhat.com bug report doesn't show me Florian's email > address. I grepped through git logs and found two associated with that > name, but didn't know if they were still accurate, or were a different > Florian. So I just went with the sentence instead. I added Florian to Cc, in case he wants to provide a preferred address. (The Red Hat Bugzilla only shows email addresses if you're logged in.) Thanks Elijah and Peff. > builtin/rev-parse.c | 8 ++++++-- > t/t6101-rev-parse-parents.sh | 8 ++++++++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c > index a1e680b5e9..a0a0ace38d 100644 > --- a/builtin/rev-parse.c > +++ b/builtin/rev-parse.c > @@ -282,6 +282,10 @@ static int try_difference(const char *arg) > struct commit *a, *b; > a = lookup_commit_reference(&start_oid); > b = lookup_commit_reference(&end_oid); > + if (!a || !b) { > + *dotdot = '.'; > + return 0; > + } > exclude = get_merge_bases(a, b); > while (exclude) { > struct commit *commit = pop_commit(&exclude); > @@ -328,12 +332,12 @@ static int try_parent_shorthands(const char *arg) > return 0; > > *dotdot = 0; > - if (get_oid_committish(arg, &oid)) { > + if (get_oid_committish(arg, &oid) || > + !(commit = lookup_commit_reference(&oid))) { > *dotdot = '^'; > return 0; > } > > - commit = lookup_commit_reference(&oid); > if (exclude_parent && > exclude_parent > commit_list_count(commit->parents)) { > *dotdot = '^'; > diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh > index 8c617981a3..7683e4a114 100755 > --- a/t/t6101-rev-parse-parents.sh > +++ b/t/t6101-rev-parse-parents.sh > @@ -214,4 +214,12 @@ test_expect_success 'rev-list merge^-1x (garbage after ^-1)' ' > test_must_fail git rev-list merge^-1x > ' > > +test_expect_success 'rev-parse $garbage^@ does not segfault' ' > + test_must_fail git rev-parse $EMPTY_TREE^@ > +' > + > +test_expect_success 'rev-parse $garbage...$garbage does not segfault' ' > + test_must_fail git rev-parse $EMPTY_TREE...$EMPTY_BLOB > +' > + > test_done -- Todd ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If the triangles were to make a God they would give him three sides. -- Montesquieu