Re: [PATCH v4] Refactor recv_sideband()

On Tue, 28 Jun 2016, Junio C Hamano wrote:

Nicolas Pitre [off-list ref] writes:

quoted

Without this, the error and remaining buffer would be reversed as 
mentioned previously.  With this, the order is restored, but a newline 
is added to unterminated lines whereas the error was simply appended to 
the output before Lukas' patch.

In any case the new behavior is probably better and I'd simply adjust 
the test expectations.

There is something else going on.  I cannot quite explain why I am
getting this failure from t5401-update-hooks.sh, for example:

    --- expect      2016-06-28 19:46:24.564937075 +0000
    +++ actual      2016-06-28 19:46:24.564937075 +0000
    @@ -9,3 +9,4 @@
     remote: STDERR post-receive
     remote: STDOUT post-update
     remote: STDERR post-update
    +remote: To ./victim.git
    not ok 12 - send-pack stderr contains hook messages

... goes and looks what v2.9.0 produces, which ends like this:

    ...
    remote: STDERR post-receive        
    remote: STDOUT post-update        
    remote: STDERR post-update        
    To ./victim.git
       e4822ab..2b65bd1  master -> master
     ! [remote rejected] tofail -> tofail (hook declined)

The test checks if lines prefixed with "remote: " match the expected
output, and the difference is an indication that the new code is
showing an extra incomplete-line "remote: " before other parts of
the code says "To ./victim.git" to report where the push is going.

Ah...  I think I know what's going on.

The leftover data in the strbuf is normally (when there is no errors) an 
unterminated line. So instead of doing:

-                       fprintf(stderr, "%s: protocol error: no band designator\n", me);
+                       strbuf_addf(&outbuf,
+                                   "\n%s: protocol error: no band designator\n",
+                                   me);

you could omit the final \n in the format string and:

-       if (outbuf.len > 0)
-               fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
+       if (outbuf.len)
+               fwrite(outbuf.buf, 1, outbuf.len, stderr);
        strbuf_release(&outbuf);

and here a \n could be added before writing out the buffer.


Nicolas