On Fri, Nov 08, 2024 at 01:24:07PM +0200, Vladimir Oltean wrote: > On Thu, Nov 07, 2024 at 11:50:34AM -0800, Jacob Keller wrote: > > +#define DECLARE_PACKED_FIELDS_S(name, buffer_sz) \ > > + const size_t __ ## name ## _buffer_sz __pf_section_s = buffer_sz; \ > > + const struct packed_field_s name[] __pf_section_s > > This will need sorting out - how to make this declaration macro > compatible with the "static" keyword. The obvious way (to group the > field array and the buffer size into a structure) doesn't work. It loses > the ARRAY_SIZE() of the fields, which is important to the pack_fields() > and unpack_fields() wrappers. > > Maybe a different tool is needed for checking that no packed fields > exceed the buffer size? Forcing the buffer size be a symbol just because > that's what modpost works with seems unnatural. > > If we knew the position of the largest field array element in C, and if > we enforced that all pack_fields() use a strong type (size included) for > the packed buffer, we'd have all information inside the pack_fields() > macro, because we only need to compare the largest field against the > buffer size. We could just have that part as a BUILD_BUG_ON() wrapped > inside the pack_fields() macro itself. And have the compile-time checks > spill over between C and modpost... > > Not to mention, pack_fields() would have one argument less: pbuflen. I was thinking something like this (attached). I still don't like modpost more than the assertions in C code, because it imposes more constraints upon the library user which didn't exist before. Though without the extra restrictions added in this patch (just ascending order for packed fields + strong type for packed buffer), I don't think the modpost variant is going to work, or is going to become extremely complex.
