Re: [PATCH 2/2] rev-parse: verify that commit looked up is not NULL

From: Todd Zullinger <tmz@pobox.com>
Date: May 23, 2018, 22:19 UTC
Message-ID: <20180523221901.GV26695@zaya.teonanacatl.net>
In-Reply-To: <20180523204613.11333-2-newren@gmail.com>

Elijah Newren wrote:

Show 6 quoted lines

> In commit 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax",
> 2008-07-26), try_parent_shorthands() was introduced to parse the special
> ^! and ^@ syntax.  However, it did not check the commit returned from
> lookup_commit_reference() before proceeding to use it.  If it is NULL,
> bail early and notify the caller that this cannot be a valid revision
> range.

Thanks. This fixes the segfault. While I was testing this, I wondered if the following cases should differ:

# f*40 $ ./git-rev-parse ffffffffffffffffffffffffffffffffffffffff^@ ; echo $? 0

# f*39 $ ./git-rev-parse fffffffffffffffffffffffffffffffffffffff^@ ; echo $? fffffffffffffffffffffffffffffffffffffff^@ fatal: ambiguous argument 'fffffffffffffffffffffffffffffffffffffff^@': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' 128

Looking a little further, this is deeper than the rev-parse handling. The difference in how these invalid refs are handled appears in 'git show' as well. With 'git show' a (different) fatal error is returned in both cases.

# f*40 $ git show ffffffffffffffffffffffffffffffffffffffff fatal: bad object ffffffffffffffffffffffffffffffffffffffff

# 39*f $ git show fffffffffffffffffffffffffffffffffffffff fatal: ambiguous argument 'fffffffffffffffffffffffffffffffffffffff': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]'

Should rev-parse return an error as well, rather than silenty succeeding?

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I refuse to spend my life worrying about what I eat. There is no
pleasure worth foregoing just for an extra three years in the
geriatric ward.
    -- John Mortimer