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

[Jeff King <peff@peff.net>]

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

> 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?

> @@ -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