Re: [PATCH net-next 3/8] lib: packing: add pack_fields() and unpack_fields()

From: Jacob Keller
Date: Thu Oct 24 2024 - 12:39:05 EST




On 10/24/2024 6:49 AM, Vladimir Oltean wrote:
> I just want to say that I don't have any alternative proposals, nor will I
> explore your sparse suggestion. I don't know enough about sparse to judge
> whether something as 'custom' as the packing API is in scope for its
> check_call_instruction() infrastructure, how well will that solution
> deal with internal kernel API changes down the line, and I don't have
> the time to learn enough to prototype something to find the maintainers'
> answer to these questions, either. I strongly prefer to have the static
> checks inside the kernel, together with the packing() API itself, so it
> can be more easily altered.
>
> Obviously you're still free to wait for more opinions and suggestions,
> or to experiment with the sparse idea yourself.
>

I also have some thought about trying to catch this in a coccinelle
script. That has the trade-off that its only caught by running the
spatch/coccinelle scripts, but it would completely eliminate the need to
modify Kbuild at all.

I'm going to try and experiment with that direction and see if its feasible.

> Honestly, my opinion is that if we can avoid messing too much with the
> top-level Kbuild file, this pretty much enters "no one really cares"
> territory, as long as the code is generated only for the pack_fields()
> users. This is, in fact, one of the reasons why the patch I attached
> earlier compiles and runs the code-gen only when PACKING_CHECK_FIELDS
> is defined.

If I can't make that work today, I'll send a v2 with the
PACKING_CHECK_FIELDS and the other cleanups applied.