On 11/13/2024 12:32 PM, Masahiro Yamada wrote: > On Mon, Nov 11, 2024 at 5:08 PM Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote: >> >> From: Vladimir Oltean <vladimir.oltean@xxxxxxx> >> >> This is new API which caters to the following requirements: >> >> - Pack or unpack a large number of fields to/from a buffer with a small >> code footprint. The current alternative is to open-code a large number >> of calls to pack() and unpack(), or to use packing() to reduce that >> number to half. But packing() is not const-correct. >> >> - Use unpacked numbers stored in variables smaller than u64. This >> reduces the rodata footprint of the stored field arrays. >> >> - Perform error checking at compile time, rather than runtime, and return >> void from the API functions. Because the C preprocessor can't generat >> variable length code (loops), we can't easily use macros to implement the >> overlap checks at compile time. >> >> Instead, check for field ordering and overlap in modpost. > > This is over-engineering. > > modpost should not be bothered just for a small library like this. > > Please do sanity checks within lib/packing.c > With the goal of maintaining compile time checks, we end up either needing to use generated macros which are O(N^2) if we allow arbitrary overlap. If we instead allow only only ascending or descending order, this would drop to O(N) which would avoid needing to have 20k lines of generated code for the case with 50. I think we could implement them without forcing drivers to specifically call the correct macro by using something like __builtin_choose_expr(), tho implementing that macro to select could be quite long. Otherwise we can fall back to either module load time checks, or go all the way back to only sanity checking at executing of pack_fields or unpack_fields.
