Re: [PATCH v4 4/4] last-modified: verify revision argument is a commit-ish

[Junio C Hamano <gitster@pobox.com>]

Toon Claes <toon@iotcl.com> writes:

> Passing a tree OID to git-last-modified(1) would trigger BUG behavior.
>
>     git last-modified HEAD^{tree}
>     BUG: builtin/last-modified.c:456: paths remaining beyond boundary in last-modified
>
> Fix this error by verifying the parsed revision is a commit-ish.

Is it "the parsed revision", or what the command line argument given
to the program?  I am wondering if it is easier to understand to
rephrase this to something like "... by ensuring that the given
revision peels to a commit".

> +		if (!repo_peel_to_type(lm->rev.repo, obj->path, 0, obj->item, OBJ_COMMIT)) {
> +			ret = error(_("revision argument '%s' is a %s, not a commit-ish"), obj->name, type_name(obj->item->type));
> +			goto out;
> +		}
> +
>  		diff_tree_oid(lm->rev.repo->hash_algo->empty_tree,
>  			      &obj->item->oid, "", &diffopt);
>  		diff_flush(&diffopt);

It is somewhat unsatisfying that the result of peeling is discarded
and diff_tree_oid() is forced to peel it down to tree again, but I
do not think of a better way offhand.