Re: [PATCH v3] Refactor recv_sideband()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 23 Jun 2016, Lukas Fleischer wrote:

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

I'd think so. This is very unlikely to happen in practice, but that 
might be useful to diagnose potential problems.

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

That makes sense to me.


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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]