Re: [PATCH v2] fetch-pack: fix object_id of exact sha1

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Gabriel,

On Sun, 28 Feb 2016, Gabriel Souza Franco wrote:

> Not the cleanest conditional I've ever written, but it should handle
> all cases correctly.

It could be much worse:

> +	if (get_oid_hex(name, &oid) ||
> +			(name[GIT_SHA1_HEXSZ] != ' ' &&
> +			 name[GIT_SHA1_HEXSZ] != '\0'))

I know developers who would write this as

	if (get_oid_hex(name, &oid) || (name[GIT_SHA1_HEXSZ] & ' '))

and not even begin to realize that this is a problem.

So I'd say your conditional is good.

Having said that, this *might* be a good opportunity to imitate the
skip_prefix() function. If there are enough similar code constructs, we
could simplify all of them by introducing the function

	skip_oid_hex(const char *str, struct object_id *oid, const char **out)

that returns 1 if and only if an oid was parsed, and stores the pointer
after the oid in "out" (skipping an additional space if there is one)?

Ciao,
Dscho