Thread (23 messages) 23 messages, 3 authors, 2017-04-20

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, char
char *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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help