On Wed, 22 Jun 2016, Nicolas Pitre wrote:
> On Wed, 22 Jun 2016, Lukas Fleischer wrote:
>
> > Before this patch, we used character buffer manipulations to split
> > messages from the sideband at line breaks and insert "remote: " at the
> > beginning of each line, using the packet size to determine the end of a
> > message. However, since it is safe to assume that diagnostic messages
> > from the sideband never contain NUL characters, we can also
> > NUL-terminate the buffer, use strpbrk() for splitting lines and use
> > format strings to insert the prefix.
> >
> > A static strbuf is used for constructing the output which is then
> > printed using a single write() call, such that the atomicity of the
> > output is preserved. See 9ac13ec (atomic write for sideband remote
> > messages, 2006-10-11) for details.
> >
> > Helped-by: Nicolas Pitre <nico@xxxxxxxxxxx>
> > Signed-off-by: Lukas Fleischer <lfleischer@xxxxxxx>
>
> The patch is buggy.
>
> Once patched, the code looks like this:
>
> case 2:
> b = buf + 1;
> /*
> * Append a suffix to each nonempty line to clear the
> * end of the screen line.
> */
> while ((brk = strpbrk(b, "\n\r"))) {
> int linelen = brk - b;
> if (linelen > 0) {
> strbuf_addf(&outbuf, "%.*s%s%c",
> linelen, b, suffix, *brk);
> } else {
> strbuf_addf(&outbuf, "%c", *brk);
> }
> xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> strbuf_reset(&outbuf);
> strbuf_addf(&outbuf, "%s", PREFIX);
> b = brk + 1;
> }
> if (*b) {
> xwrite(STDERR_FILENO, outbuf.buf, outbuf.len);
> /* Incomplete line, skip the next prefix. */
> strbuf_reset(&outbuf);
> }
> continue;
>
> You are probably missing a strbuf_addf() before the last xwrite().
In fact, you could simply append the partial line to the strbuf and make
it the prefix for the next packet rather than writing a partial line.
You'd only have to write a partial line before leaving the function if
the strbuf is not empty at that point.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html