Re: [PATCH v5 01/35] pkt-line: introduce packet_read_with_status
- From
- Brandon Williams <bmwill@google.com>
- Date
- Mar 15, 2018, 17:28 UTC
- Message-ID
- <20180315172848.GB174336@google.com>
- In-Reply-To
- <xmqq1sgmz6j0.fsf@gitster-ct.c.googlers.com>
On 03/14, Junio C Hamano wrote:
Show 36 quoted lines
> Brandon Williams <bmwill@google.com> 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