Re: [PATCH v3] Refactor recv_sideband()

From: Lukas Fleischer <lfleischer@lfos.de>
Date: Jun 23, 2016, 17:35 UTC
Message-ID: <146670333753.555.4852543207010532908@typhoon.lan>
In-Reply-To: <alpine.LFD.2.20.1606221831100.2550@knanqh.ubzr>

On Thu, 23 Jun 2016 at 00:47:39, Nicolas Pitre wrote:

Show 15 quoted lines

> 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.