Re: [PATCH v5 1/8] pkt-line: add packet_read_line_gently()
flat view
From: Lars Schneider <hidden>
Date: 2017-04-09 19:35:06
quoted hunk ↗ jump to hunk
On 07 Apr 2017, at 14:03, Ben Peart [off-list ref] wrote: Add packet_read_line_gently() to enable reading a line without dying on EOF. Signed-off-by: Ben Peart <redacted> --- pkt-line.c | 12 ++++++++++++ pkt-line.h | 10 ++++++++++ 2 files changed, 22 insertions(+)diff --git a/pkt-line.c b/pkt-line.c index d4b6bfe076..58842544b4 100644 --- a/pkt-line.c +++ b/pkt-line.c@@ -323,6 +323,18 @@ char *packet_read_line(int fd, int *len_p)return packet_read_line_generic(fd, NULL, NULL, len_p); } +int packet_read_line_gently(int fd, int *dst_len, char** dst_line) +{ + int len = packet_read(fd, NULL, NULL, + packet_buffer, sizeof(packet_buffer), + PACKET_READ_CHOMP_NEWLINE|PACKET_READ_GENTLE_ON_EOF);
Really minor nit: you could align the parameters according to fd similar to the "packet_read_line_generic" code (tab width 8).
quoted hunk ↗ jump to hunk
+ if (dst_len) + *dst_len = len; + if (dst_line) + *dst_line = (len > 0) ? packet_buffer : NULL;
Minor nit: The explicit check "len > 0" is necessary here as len can be "-1". The original "packet_read_line_generic" just checks for "len". I think it would be nice if the code would be consistent and both would check "len > 0".
quoted hunk ↗ jump to hunk
+ return len; +} + char *packet_read_line_buf(char **src, size_t *src_len, int *dst_len) { return packet_read_line_generic(-1, src, src_len, dst_len);diff --git a/pkt-line.h b/pkt-line.h index 18eac64830..12b18991f6 100644 --- a/pkt-line.h +++ b/pkt-line.h@@ -74,6 +74,16 @@ int packet_read(int fd, char **src_buffer, size_t *src_len, charchar *packet_read_line(int fd, int *size); /* + * Convenience wrapper for packet_read that sets the PACKET_READ_GENTLE_ON_EOF + * and CHOMP_NEWLINE options. The return value specifies the number of bytes + * read into the buffer or -1 on truncated input. if the *dst_line parameter
s/if/If/
+ * is not NULL it will return NULL for a flush packet and otherwise points to
This sentences is a bit confusing to me. Maybe: If the *dst_line parameter is not NULL, then it will point to a static buffer (that may be overwritten by subsequent calls) or it will return NULL for a flush packet. ... feel free to completely ignore this as I am no native speaker.
quoted hunk ↗ jump to hunk
+ * a static buffer (that may be overwritten by subsequent calls). If the size + * parameter is not NULL, the length of the packet is written to it. + */ +int packet_read_line_gently(int fd, int *size, char** dst_line); + +/* * Same as packet_read_line, but read from a buf rather than a descriptor; * see packet_read for details on how src_* is used. */
If you send another round then you could address the minor nits. If not, then this patch as it is looks good to me. Thanks, Lars