On Tue, 2 Jan 2018 16:18:03 -0800
Brandon Williams <bmw...@google.com> wrote:
> -int packet_read(int fd, char **src_buf, size_t *src_len,
> - char *buffer, unsigned size, int options)
> +enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
> size_t *src_len,
> + char *buffer, unsigned size,
> int *pktlen,
> + int options)
> {
> - int len, ret;
> + int len;
> char linelen[4];
>
> - ret = get_packet_data(fd, src_buf, src_len, linelen, 4, options);
> - if (ret < 0)
> - return ret;
> + if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0)
> + return PACKET_READ_EOF;
> +
> len = packet_length(linelen);
> if (len < 0)
> die("protocol error: bad line length character: %.4s", linelen);
> - if (!len) {
> +
> + if (len == 0) {
This change (replacing "!len" with "len == 0") is unnecessary, I think.
> packet_trace("0000", 4, 0);
> - return 0;
> + return PACKET_READ_FLUSH;
> + } else if (len >= 1 && len <= 3) {
> + die("protocol error: bad line length character: %.4s", linelen);
> }
This seems to be more of a "bad line length" than a "bad line length
character".
Also, some of the checks are redundant. Above, it is probably better to
delete "len >= 1", and optionally write "len < 4" instead of "len <= 3"
(to emphasize that the subtraction of 4 below does not result in a
negative value).
> +
> len -= 4;
> - if (len >= size)
> + if ((len < 0) || ((unsigned)len >= size))
> die("protocol error: bad line length %d", len);
The "len < 0" check is redundant.
> - ret = get_packet_data(fd, src_buf, src_len, buffer, len, options);
> - if (ret < 0)
> - return ret;
> +
> + if (get_packet_data(fd, src_buffer, src_len, buffer, len, options) < 0)
> + return PACKET_READ_EOF;
>
> if ((options & PACKET_READ_CHOMP_NEWLINE) &&
> len && buffer[len-1] == '\n')
