Re: [PATCH v4] Refactor recv_sideband()
From: Junio C Hamano <hidden>
Date: 2016-06-29 03:00:44
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-07-07 · Re: [PATCH v4] Refactor recv_sideband() · Nicolas Pitre <nico@fluxnic.net>
- 2016-07-06 · Re: [PATCH v4] Refactor recv_sideband() · Junio C Hamano <hidden>
- 2016-07-05 · Re: [PATCH v4] Refactor recv_sideband() · Nicolas Pitre <nico@fluxnic.net>
- 2016-07-01 · Re: [PATCH v4] Refactor recv_sideband() · Junio C Hamano <hidden>
- 2016-06-30 · Re: [PATCH v4] Refactor recv_sideband() · Lukas Fleischer <hidden>
Junio C Hamano [off-list ref] writes:
It's just that if you take the latter, then the conditional after the loop exits (i.e. the last transmission was an incomplete line) cannot be "is outbuf empty?", as your base state is "has PREFIX and can never be empty". I was working back from that if statement.
Let's try this again. How does this look? In this version: - "outbuf" is where we keep the (possibly partial) data collected to be eventually shown; - output of pending (possibly partial) data is handled by a helper function drain(). It is responsible for prepending of the PREFIX, which is treated purely as a cosmetic thing. It also is responsible for completing an incomplete line at the end of the transmission (e.g. flushing of the buffered input upon reception of the emergency exit packet). - locally generated errors go directly to fprintf(stderr), bypassing outbuf (hence drain()). sideband.c | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/sideband.c b/sideband.c
index 226a8c2..6873137 100644
--- a/sideband.c
+++ b/sideband.c@@ -18,6 +18,16 @@ #define ANSI_SUFFIX "\033[K" #define DUMB_SUFFIX " " +static void drain(struct strbuf *outbuf) +{ + if (!outbuf->len) + return; + strbuf_splice(outbuf, 0, 0, PREFIX, strlen(PREFIX)); + strbuf_complete_line(outbuf); + fwrite(outbuf->buf, 1, outbuf->len, stderr); + strbuf_reset(outbuf); +} + int recv_sideband(const char *me, int in_stream, int out) { const char *term, *suffix;
@@ -26,20 +36,21 @@ int recv_sideband(const char *me, int in_stream, int out) const char *b, *brk; int retval = 0; - strbuf_addf(&outbuf, "%s", PREFIX); term = getenv("TERM"); if (isatty(2) && term && strcmp(term, "dumb")) suffix = ANSI_SUFFIX; else suffix = DUMB_SUFFIX; - while (retval == 0) { + while (!retval) { int band, len; len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 0); if (len == 0) break; if (len < 1) { - fprintf(stderr, "%s: protocol error: no band designator\n", me); + drain(&outbuf); + fprintf(stderr, "%s: protocol error: no band designator\n", + me); retval = SIDEBAND_PROTOCOL_ERROR; break; }
@@ -48,7 +59,8 @@ int recv_sideband(const char *me, int in_stream, int out) len--; switch (band) { case 3: - fprintf(stderr, "%s%s\n", PREFIX, buf + 1); + drain(&outbuf); + strbuf_addf(&outbuf, "%s\n", buf + 1); retval = SIDEBAND_REMOTE_ERROR; break; case 2:
@@ -58,13 +70,12 @@ int recv_sideband(const char *me, int in_stream, int out) * Append a suffix to each nonempty line to clear the * end of the screen line. * - * The output is accumulated in a buffer and each line - * is printed to stderr using fprintf() with a single - * conversion specifier. This is a "best effort" - * approach to supporting both inter-process atomicity - * (single conversion specifiers are likely to end up - * in single atomic write() system calls) and the ANSI - * control code emulation under Windows. + * The output is accumulated in a buffer and + * each line is printed to stderr using + * fwrite(3). This is a "best effort" + * approach to support inter-process atomicity + * (single fwrite(3) call is likely to end up + * in single atomic write() system calls). */ while ((brk = strpbrk(b, "\n\r"))) { int linelen = brk - b;
@@ -75,11 +86,7 @@ int recv_sideband(const char *me, int in_stream, int out) } else { strbuf_addf(&outbuf, "%c", *brk); } - fprintf(stderr, "%.*s", (int)outbuf.len, - outbuf.buf); - strbuf_reset(&outbuf); - strbuf_addf(&outbuf, "%s", PREFIX); - + drain(&outbuf); b = brk + 1; }
@@ -90,6 +97,7 @@ int recv_sideband(const char *me, int in_stream, int out) write_or_die(out, buf + 1, len); break; default: + drain(&outbuf); fprintf(stderr, "%s: protocol error: bad band #%d\n", me, band); retval = SIDEBAND_PROTOCOL_ERROR;
@@ -97,8 +105,7 @@ int recv_sideband(const char *me, int in_stream, int out) } } - if (outbuf.len > 0) - fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf); + drain(&outbuf); strbuf_release(&outbuf); return retval; }