Re: [PATCH v7 06/10] pkt-line: add functions to read/write flush terminated packet streams
From: Stefan Beller <hidden>
Date: 2016-09-08 21:49:38
On Thu, Sep 8, 2016 at 11:21 AM, [off-list ref] wrote:
From: Lars Schneider <redacted> write_packetized_from_fd() and write_packetized_from_buf() write a stream of packets. All content packets use the maximal packet size except for the last one. After the last content packet a `flush` control packet is written.
I presume we need both write_* things in a later patch; can you clarify why we need both of them?
quoted hunk ↗ jump to hunk
+ if (paket_len < 0) { + if (oldalloc == 0) + strbuf_release(sb_out);
So if old alloc is 0, we release it, which is documented as /** * Release a string buffer and the memory it used. You should not use the * string buffer after using this function, unless you initialize it again. */
+ else + strbuf_setlen(sb_out, oldlen);
Otherwise we just set the length back, such that it looks like before. So as a caller the strbuf is in a different state in case of error depending whether the strbuf already had some data in it. I think it would be better if we only did `strbuf_setlen(sb_out, oldlen);` here, such that the caller can strbuf_release it unconditionally. Or to make things more confusing, you could use strbuf_reset in case of 0, as that is a strbuf_setlen internally. ;)
quoted hunk ↗ jump to hunk
@@ -77,6 +79,11 @@ char *packet_read_line(int fd, int *size); */ char *packet_read_line_buf(char **src_buf, size_t *src_len, int *size); +/* + * Reads a stream of variable sized packets until a flush packet is detected.
Strictly speaking we read until a packet of size 0 appears, but as per the implementation of packet_read we cannot distinguish between "0000" and "0004", i.e. an empty non-flush packet. So I think we're fine both in the implementation as well as the documentation here.