Subject: Re: [RFC PATCH v2 01/16] pkt-line: Add strbuf based functions

From: Johannes Sixt <[email-masked]>
Date: Tue, 13 Oct 2009 09:29:22 +0200

Shawn O. Pearce schrieb:
> -int packet_read_line(int fd, char *buffer, unsigned size)
> +static int packet_length(unsigned *ret_len, const char *linelen)
>  {
>  	int n;
> -	unsigned len;
> -	char linelen[4];
> -
> -	safe_read(fd, linelen, 4);
> +	unsigned len = 0;
>  
> -	len = 0;
>  	for (n = 0; n < 4; n++) {
>  		unsigned char c = linelen[n];
>  		len <<= 4;
> @@ -96,8 +116,20 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  			len += c - 'A' + 10;
>  			continue;
>  		}
> -		die("protocol error: bad line length character");
> +		return -1;
>  	}
> +	*ret_len = len;
> +	return 0;
> +}

len can be signed: Valid lengths fit into a signed int. Then you can
'return len;' on success and 'return -1;' on failure and don't need return
the result by reference. packet_read_line() ultimately converts it to int
anyway:

> +int packet_read_line(int fd, char *buffer, unsigned size)
> +{
> +	unsigned len;
> +	char linelen[4];
> +
> +	safe_read(fd, linelen, 4);
> +	if (packet_length(&len, linelen))
> +		die("protocol error: bad line length character");
>  	if (!len)
>  		return 0;
>  	len -= 4;
> @@ -107,3 +139,28 @@ int packet_read_line(int fd, char *buffer, unsigned size)
>  	buffer[len] = 0;
>  	return len;
>  }

-- Hannes