Re: [PATCH v9 10/14] pkt-line: add functions to read/write flush terminated packet streams
From: Junio C Hamano <hidden>
Date: 2016-10-04 19:53:59
larsxschneider@gmail.com writes:
quoted hunk
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. read_packetized_to_strbuf() reads arbitrary sized packets until it detects a `flush` packet. Signed-off-by: Lars Schneider <redacted> Signed-off-by: Junio C Hamano <redacted> --- pkt-line.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ pkt-line.h | 8 ++++++++ 2 files changed, 77 insertions(+)diff --git a/pkt-line.c b/pkt-line.c index 3fd4dc0..8ffde22 100644 --- a/pkt-line.c +++ b/pkt-line.c@@ -196,6 +196,47 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...) va_end(args); } +int write_packetized_from_fd(int fd_in, int fd_out) +{ + static char buf[LARGE_PACKET_DATA_MAX]; + int err = 0; + ssize_t bytes_to_write; + + while (!err) { + bytes_to_write = xread(fd_in, buf, sizeof(buf)); + if (bytes_to_write < 0) + return COPY_READ_ERROR; + if (bytes_to_write == 0) + break; + err = packet_write_gently(fd_out, buf, bytes_to_write); + } + if (!err) + err = packet_flush_gently(fd_out); + return err; +}
OK.
+int write_packetized_from_buf(const char *src_in, size_t len, int fd_out)
+{
+ static char buf[LARGE_PACKET_DATA_MAX];
+ int err = 0;
+ size_t bytes_written = 0;
+ size_t bytes_to_write;
+
+ while (!err) {
+ if ((len - bytes_written) > sizeof(buf))
+ bytes_to_write = sizeof(buf);
+ else
+ bytes_to_write = len - bytes_written;
+ if (bytes_to_write == 0)
+ break;
+ err = packet_write_gently(fd_out, src_in + bytes_written, bytes_to_write);
+ bytes_written += bytes_to_write;
+ }
+ if (!err)
+ err = packet_flush_gently(fd_out);
+ return err;
+}Hmph, what is buf[] used for, other than its sizeof() taken to yield a constant LARGE_PACKET_DATA_MAX?
quoted hunk
@@ -305,3 +346,31 @@ 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); } + +ssize_t read_packetized_to_strbuf(int fd_in, struct strbuf *sb_out) +{ + int packet_len; + + size_t orig_len = sb_out->len; + size_t orig_alloc = sb_out->alloc; + + for (;;) { + strbuf_grow(sb_out, LARGE_PACKET_DATA_MAX); + packet_len = packet_read(fd_in, NULL, NULL, + // TODO: explain + 1
No // C99 comment please. And I agree that the +1 needs to be explained.
+ sb_out->buf + sb_out->len, LARGE_PACKET_DATA_MAX+1, + PACKET_READ_GENTLE_ON_EOF); + if (packet_len <= 0) + break;
Hmph. So at the end of a data stream, we ask packet_read() to read 64kB or so, packet_read() gets the packet length by calling get_packet_data() and then another get_packet_data() reads that much and return. What happens during the next round? The first call to get_packet_data() in packet_read() will find that the stream has ended and returns -1, which is stored in packet_len here? But then the data is discarded after the loop when packet_len is negative. I must be missing something. Is the other side always supposed to give a flush packet or something? Perhaps that is what is happening here. If so, I am OK with that, even though it somehow sounds a bit wasteful.