On Wed, Feb 06, 2019 at 11:29:03AM -0800, Josh Steadmon wrote:
> > + packet_reader_init(&reader, -1, d->buf, d->len,
> > + PACKET_READ_CHOMP_NEWLINE |
> > + PACKET_READ_DIE_ON_ERR_PACKET);
> > + if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
> > + die("invalid server response; expected service, got flush
> > packet");
>
> This can also trigger on an EOF or a delim packet, should we clarify the
> error message?
Maybe, though I'd prefer to do it as a patch on top; these lines were
moved straight from the existing code.
> > + if (skip_prefix(reader.line, "# service=", &p) && !strcmp(p, service)) {
> > + /*
> > + * The header can include additional metadata lines, up
> > + * until a packet flush marker. Ignore these now, but
> > + * in the future we might start to scan them.
> > + */
> > + for (;;) {
> > + packet_reader_read(&reader);
> > + if (reader.pktlen <= 0) {
> > + break;
> > + }
> > + }
>
> Could we make this loop cleaner as:
>
> while (packet_reader_read(&reader) != PACKET_READ_NORMAL)
> ;
Likewise here.
-Peff
