Subject: Re: [PATCH v5 01/35] pkt-line: introduce packet_read_with_status

From: Brandon Williams <[email-masked]>
Date: Thu, 15 Mar 2018 10:28:48 -0700

On 03/14, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > +/*
> > + * Read a packetized line into a buffer like the 'packet_read()' function but
> > + * returns an 'enum packet_read_status' which indicates the status of the read.
> > + * The number of bytes read will be assigined to *pktlen if the status of the
> > + * read was 'PACKET_READ_NORMAL'.
> > + */
> > +enum packet_read_status {
> > +	PACKET_READ_EOF,
> > +	PACKET_READ_NORMAL,
> > +	PACKET_READ_FLUSH,
> > +};
> 
> EOF was -1 and NORMAL was 0 in the previous round; do we need to
> read through all the invocations of functions that return this type
> and make sure there is no "while (such_a_function())" that used to see
> if we read NORMAL that is left un-updated?
> 
> I just have gone thru all the hits from
> 
>  $ git grep -n -e packet_erad_with_status -e packet_reader_read -e packet_reader_peek
> 
> There are a few
> 
> 	switch (packet_reader_peek())
> 
> which by definition we do not have to worry about.  Then majority of
> what could be problematic are of the form:
> 
> 	while (packet_reader_read() == PACKET_READ_NORMAL)
> 
> and they were this way even in the previous version, so it seems
> quite alright.
> 
> Will replace.  Thanks.

A reviewer in the previous round found that it was unnecessary to have
EOF start at -1, so per their comments I got rid of that.

-- 
Brandon Williams