On Mon, 2026-08-10 at 13:33 -0700, Jakub Kicinski wrote:
> On Mon, 10 Aug 2026 11:42:36 +0000 Cosmin Ratiu wrote:
> > > > +#define cmd(_name,
> > > > _extra_sz) \
> > > > ({
> > > >
> > > > \
> > > > ssize_t sz =
> > > > sizeof(_name); \
> > > > - bool match = n >= sz
> > > > && !memcmp(buf, _name, sz); \
> > > > + bool match = off >= sz + (_extra_sz)
> > > > && \
> > > > + !memcmp(buf, _name,
> > > > sz); \
> > >
> > > Please don't replace off by n.
> >
> > I actually replaced n by off. Did you mean "don't replace n by
> > off"?
>
> Of course!
>
> > Because the two are identical after the recv error checking. I
> > don't
> > see the point of maintaining two variables when one suffices.
> > Either n
> > or off can describe the number of bytes in buf. off is static and
> > persists across chunks, while n is temporary.
>
> The variables have semantic meaning, which makes it easier to follow
> the code.
>
> > So to clarify, do you want to keep both?
>
> Yes.
I'm sorry, but the code is wrong.
cmd() matches on n (which never changes!) but __consume() only
decrements off. Strap a loop on top and the result is just confusion
and a source of bugs (e.g. the one I'm trying to fix with this patch).
A single variable should be used to keep track of the length of data
left to parse.
Cosmin.
