Re: [PATCH v2] bisect: don't use invalid oid as rev when starting

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 24, 2020, 19:56 UTC
Message-ID: <xmqqk0wjlzj8.fsf@gitster.c.googlers.com>
In-Reply-To: <xmqqsgb7m2bq.fsf@gitster.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> We may need to audit recent additions of get_oid_committish() calls
> in our codebase.  I suspect there may be other instances of the same
> mistake.

Interim progress report. I've only looked at files that use get_oid_treeish() but audited all uses of get_oid_*ish() in them.

The results are as follows.

 * builtin/reset.c::parse_args() makes get_oid_committish() and
   get_oid_treeish() only to discard the object name, because it
   wants to ensure the args can be peeled down to such.  OK.

 * builtin/reset.c::cmd_reset() applies get_oid_committish() and
   get_oid_treeish() to the result taken from the above, but then
   uses lookup_commit_reference() and parse_tree_indirect() to peel
   the result to the desired type.  OK.

 * notes.c::init_notes() uses get_oid_treeish() to validate that the
   notes ref can be read as a tree, and then uses get_tree_entry()
   on it, which in turn uses read_object_with_reference() for
   tree_type so it tolerates a commit object.  OK.

I didn't audit the following hits of get_oid_committish(). There might be a similar mistake as you made in v2, or there may not be.

I am undecided if I should just move on, marking them as left-over-bits ;-)

builtin/blame.c: if (get_oid_committish(i->string, &oid)) builtin/checkout.c: repo_get_oid_committish(the_repository, branch->name, &branch->oid); builtin/rev-parse.c: if (!get_oid_committish(start, &start_oid) && !get_oid_committish(end, &end_oid)) { builtin/rev-parse.c: if (get_oid_committish(arg, &oid) || commit.c: if (get_oid_committish(name, &oid)) revision.c: if (get_oid_committish(arg, &oid)) sequencer.c: !get_oid_committish(buf.buf, &oid)) sha1-name.c: st = repo_get_oid_committish(r, sb.buf, &oid_tmp); sha1-name.c: if (repo_get_oid_committish(r, dots[3] ? (dots + 3) : "HEAD", &oid_tmp)) sha1-name.c:int repo_get_oid_committish(struct repository *r, t/helper/test-reach.c: if (get_oid_committish(buf.buf + 2, &oid))