On Thu, Sep 8, 2016 at 11:21 AM, <larsxschneider@xxxxxxxxx> wrote:
> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>
> 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?
> + 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. ;)
> @@ -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.