Re: [PATCH net-next V2 12/13] selftests: drv-net: psp: Fix responder parsing

From: Cosmin Ratiu

Date: Tue Aug 11 2026 - 08:44:38 EST


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.