Re: [PATCH 01/26] pkt-line: introduce packet_read_with_status
From: Jonathan Tan <hidden>
Date: 2018-01-09 18:04:18
On Tue, 2 Jan 2018 16:18:03 -0800 Brandon Williams [off-list ref] 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')