Re: [PATCH v3 02/35] pkt-line: introduce struct packet_reader

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 27, 2018, 05:57 UTC
Message-ID: <20180227055746.GC65699@aiede.svl.corp.google.com>
In-Reply-To: <20180207011312.189834-3-bmwill@google.com>

Hi,

Brandon Williams wrote:

Show 13 quoted lines

> Sometimes it is advantageous to be able to peek the next packet line
> without consuming it (e.g. to be able to determine the protocol version
> a server is speaking).  In order to do that introduce 'struct
> packet_reader' which is an abstraction around the normal packet reading
> logic.  This enables a caller to be able to peek a single line at a time
> using 'packet_reader_peek()' and having a caller consume a line by
> calling 'packet_reader_read()'.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  pkt-line.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  pkt-line.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)

I like it!

The questions and nits from https://public-inbox.org/git/20180213004937.GB42272@aiede.svl.corp.google.com/ still apply. In particular, the ownership of the buffers inside the 'struct packet_reader' is still unclear; could the packet_reader create its own (strbuf) buffers so that the contract around them (who is allowed to write to them; who is responsible for freeing them) is more obvious?

Thanks, Jonathan