From: Junio C Hamano <gitster@pobox.com>
To: larsxschneider@gmail.com
Cc: git@vger.kernel.org, ramsay@ramsayjones.plus.com,
jnareb@gmail.com, j6t@kdbg.org, tboegi@web.de, peff@peff.net,
mlbright@gmail.com
Subject: Re: [PATCH v9 10/14] pkt-line: add functions to read/write flush terminated packet streams
Date: Tue, 04 Oct 2016 12:53:50 -0700 [thread overview]
Message-ID: <xmqq8tu3ubzl.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20161004125947.67104-11-larsxschneider@gmail.com> (larsxschneider@gmail.com's message of "Tue, 4 Oct 2016 14:59:43 +0200")
larsxschneider@gmail.com writes:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> 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 <larsxschneider@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 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?
> @@ -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.
next prev parent reply other threads:[~2016-10-04 19:53 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-04 12:59 [PATCH v9 00/14] Git filter protocol larsxschneider
2016-10-04 12:59 ` [PATCH v9 01/14] convert: quote filter names in error messages larsxschneider
2016-10-04 12:59 ` [PATCH v9 02/14] convert: modernize tests larsxschneider
2016-10-04 12:59 ` [PATCH v9 03/14] run-command: move check_pipe() from write_or_die to run_command larsxschneider
2016-10-04 12:59 ` [PATCH v9 04/14] run-command: add wait_on_exit larsxschneider
2016-10-04 19:30 ` Junio C Hamano
2016-10-05 20:57 ` Lars Schneider
2016-10-05 21:12 ` Junio C Hamano
2016-10-06 9:32 ` Johannes Schindelin
2016-10-06 9:35 ` Lars Schneider
2016-10-06 15:53 ` Junio C Hamano
2016-10-04 12:59 ` [PATCH v9 05/14] pkt-line: rename packet_write() to packet_write_fmt() larsxschneider
2016-10-04 12:59 ` [PATCH v9 06/14] pkt-line: extract set_packet_header() larsxschneider
2016-10-04 12:59 ` [PATCH v9 07/14] pkt-line: add packet_write_fmt_gently() larsxschneider
2016-10-04 12:59 ` [PATCH v9 08/14] pkt-line: add packet_flush_gently() larsxschneider
2016-10-04 12:59 ` [PATCH v9 09/14] pkt-line: add packet_write_gently() larsxschneider
2016-10-04 19:33 ` Junio C Hamano
2016-10-05 19:06 ` Lars Schneider
2016-10-06 17:25 ` Junio C Hamano
2016-10-04 12:59 ` [PATCH v9 10/14] pkt-line: add functions to read/write flush terminated packet streams larsxschneider
2016-10-04 19:53 ` Junio C Hamano [this message]
2016-10-04 19:58 ` Junio C Hamano
2016-10-05 17:35 ` Lars Schneider
2016-10-04 12:59 ` [PATCH v9 11/14] convert: make apply_filter() adhere to standard Git error handling larsxschneider
2016-10-04 12:59 ` [PATCH v9 12/14] convert: prepare filter.<driver>.process option larsxschneider
2016-10-04 12:59 ` [PATCH v9 13/14] convert: add " larsxschneider
2016-10-04 12:59 ` [PATCH v9 14/14] contrib/long-running-filter: add long running filter example larsxschneider
2016-10-05 22:31 ` [PATCH v9 00/14] Git filter protocol Jakub Narębski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq8tu3ubzl.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=j6t@kdbg.org \
--cc=jnareb@gmail.com \
--cc=larsxschneider@gmail.com \
--cc=mlbright@gmail.com \
--cc=peff@peff.net \
--cc=ramsay@ramsayjones.plus.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).