Subject: Re: [PATCH v2 1/8] test-pkt-line: add unpack-sideband subcommand

From: Brandon Williams <[email-masked]>
Date: Thu, 14 Jun 2018 12:21:50 -0700

On 06/14, Stefan Beller wrote:
> On Wed, Jun 13, 2018 at 2:39 PM Brandon Williams <[email-masked]> wrote:
> >
> > Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
> > enable unpacking packet line data sent multiplexed using a sideband.
> >
> > Signed-off-by: Brandon Williams <[email-masked]>
> > ---
> >  t/helper/test-pkt-line.c | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
> > index 0f19e53c7..2a55ffff1 100644
> > --- a/t/helper/test-pkt-line.c
> > +++ b/t/helper/test-pkt-line.c
> > @@ -1,3 +1,4 @@
> > +#include "cache.h"
> >  #include "pkt-line.h"
> >
> >  static void pack_line(const char *line)
> > @@ -48,6 +49,40 @@ static void unpack(void)
> >         }
> >  }
> >
> > +static void unpack_sideband(void)
> > +{
> > +       struct packet_reader reader;
> > +       packet_reader_init(&reader, 0, NULL, 0,
> > +                          PACKET_READ_GENTLE_ON_EOF |
> > +                          PACKET_READ_CHOMP_NEWLINE);
> > +
> > +       while (packet_reader_read(&reader) != PACKET_READ_EOF) {
> > +               int band;
> > +               int fd;
> > +
> > +               switch (reader.status) {
> > +               case PACKET_READ_EOF:
> > +                       break;
> > +               case PACKET_READ_NORMAL:
> > +                       band = reader.line[0] & 0xff;
> > +                       if (band == 1)
> > +                               fd = 1;
> > +                       else
> > +                               fd = 2;
> > +
> > +                       write_or_die(fd, reader.line+1, reader.pktlen-1);
> 
> white space around + and - ?

Will fix.

> 
> > +
> > +                       if (band == 3)
> > +                               die("sind-band error");
> 
> s/sind/side/ ?

Thanks for catching this.

> 
> What values for band are possible?
> e.g. band==4 would also just write to fd=1;
> but I suspect we don't want that, yet.
> 
> So maybe
> 
>     band = reader.line[0] & 0xff;
>     if (band < 1 || band > 2)
>         die("unexpected side band %d", band)
>     fd = band;
> 
> instead?

Yeah that's must cleaner logic.

-- 
Brandon Williams