Re: [PATCH v3] Refactor recv_sideband()

[Lukas Fleischer <lfleischer@lfos.de>]

On Thu, 23 Jun 2016 at 00:47:39, Nicolas Pitre wrote:
> On Wed, 22 Jun 2016, Nicolas Pitre wrote:
> [...]
> >                 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.

True. And I like that solution.

Thinking about your last sentence, do we care about printing an
incomplete line at the end of the communication at all? If so, do we
need to print such a line on every return path (i.e. on protocol and
remote errors as well)? If we do, and if we want to implement partial
line handling the way you suggested, we should probably print that final
line from a common return path. And if we add such a path, we could
reconsider using a non-static strbuf as well, since we could simply
strbuf_release() the output buffer in that common code block. Opinions?

I will wait for more comments and submit v4 in a couple of days.