From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: last bit of rde_attr_parse ibuf cleanup
To: tech@openbsd.org
Date: Tue, 30 Jan 2024 14:35:00 +0100

> > I don't like this manually advancing the nseg buffer. Should ndata not
> > be wrapped in an ibuf to which you add the data from buf, padding with
> > two zeroes for each segment? This will be a bit more code, but you'd get
> > a correct size in a single loop.
> 
> Totally changed to code to return a struct ibuf *, so we can use
> ibuf_open() and ibuf_add() this makes the code a lot simpler.

Lovely. It didn't occur to me that you could just use ibuf_add_n32().

> > > +		    ibuf_get_n8(&buf, &seg_len) == -1) {
> > > +			free(ndata);
> > > +			return (NULL);
> > > +		}
> > > +		*nseg++ = seg_len;
> > 
> > Again, I would probably error on 0 seg_len here.
> > 
> > >  		for (; seg_len > 0; seg_len--) {
> > 
> > Might be worth using an extra ibuf for the segment.
> 
> Not sure if that makes the code better. I skipped that for now. 

Fine with me. With CBS/CBB we generally do everything to avoid any
explicit pointer or length fiddling, but I see your point.

ok tb

> Need to check if aspath_inflate() is actually excercised in regress. It is
> a code path that is far less travelled then the default 4byte ASnum one.

That would also be nice.