Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King wrote:

> --- a/remote-curl.c
> +++ b/remote-curl.c
[...]
> @@ -155,11 +166,13 @@ static struct discovery* discover_refs(const char *service)
[...]
> -		strbuf_reset(&buffer);
> -		while (packet_get_line(&buffer, &last->buf, &last->len) > 0)
> -			strbuf_reset(&buffer);
> +		if (read_packets_until_flush(&last->buf, &last->len) < 0)

Style nit: this made me wonder "What would it mean if
read_packets_until_flush() > 0?"  Since the convention for this
function is "0 for success", I would personally find

		if (read_packets_until_flush(...))
			handle error;

easier to read.

> +			die("smart-http metadata lines are invalid at %s",
> +			    refs_url);

Especially given that other clients would be likely to run into
trouble in the same situation, as long as this cooks in "next" for a
suitable amount of time to catch bad servers, it looks like a good
idea.

Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]