From: Brandon Williams Date: Thu, 15 Mar 2018 17:28:48 GMT Subject: Re: [PATCH v5 01/35] pkt-line: introduce packet_read_with_status Message-ID: <20180315172848.GB174336@google.com> In-Reply-To: On 03/14, Junio C Hamano wrote: > Brandon Williams 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