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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 29, 2016, 08:30 UTC
Message-ID: <alpine.DEB.2.20.1602290912260.3152@virtualbox>
In-Reply-To: <1456698144-11519-1-git-send-email-gabrielfrancosouza@gmail.com>

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