Re: [PATCH v2 07/21] builtin/receive-pack: convert portions to struct object_id

From: Jeff King <peff@peff.net>
Date: Mar 28, 2017, 07:07 UTC
Message-ID: <20170328070712.p2o7wziysy7n2vxk@sigill.intra.peff.net>
In-Reply-To: <20170326160143.769630-8-sandals@crustytoothpaste.net>

On Sun, Mar 26, 2017 at 04:01:29PM +0000, brian m. carlson wrote:

Show 7 quoted lines

> Convert some hardcoded constants into uses of parse_oid_hex.
> Additionally, convert all uses of struct command, and miscellaneous
> other functions necessary for that.  This work is necessary to be able
> to convert sha1_array_append later on.
> 
> To avoid needing to specify a constant, reject shallow lines with the
> wrong length instead of simply ignoring them.

It took me a while to find it. This is the switch from "len == 48" to "len > 8" when matching "shallow" lines. I think this makes sense.

> Note that in queue_command we are guaranteed to have a NUL-terminated
> buffer or at least one byte of overflow that we can safely read, so the
> linelen check can be elided.  We would die in such a case, but not read
> invalid memory.

I think linelen is always just strlen(line). Since the queue_command function no longer cares about it, perhaps we can just omit it?

Show 12 quoted lines

> @@ -1541,12 +1541,12 @@ static struct command *read_head_info(struct sha1_array *shallow)
>  		if (!line)
>  			break;
>  
> -		if (len == 48 && starts_with(line, "shallow ")) {
> -			unsigned char sha1[20];
> -			if (get_sha1_hex(line + 8, sha1))
> +		if (len > 8 && starts_with(line, "shallow ")) {
> +			struct object_id oid;
> +			if (get_oid_hex(line + 8, &oid))
>  				die("protocol error: expected shallow sha, got '%s'",
>  				    line + 8);

This would be much nicer as:

  if (skip_prefix(line, "shallow ", &hex))

It's probably best to keep to one type of cleanup at a time here. I'm just making a mental note.

-Peff