Junio C Hamano <gits...@pobox.com> 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;
 }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to