On 11/8/2024 10:31 AM, Vladimir Oltean wrote: > 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. To me, the restrictions seem acceptable. You can always wrap an arbitrary void buffer into a sized type via a structure. I do agree that modpost isn't perfect with respect to the C code, but I dislike the sheer size of the macros generated and the complexity added to the build system. My preferred solution is unfortunately not available in C: having the compiler evaluate a constant function. There is a C++ extension that does that, but it doesn't seem like GCC has enabled such support in the C even as an extension. I'd prefer to keep allowing both ascending and descending order, because I think those are both valid orderings depending on how you're thinking about the fields (little vs big endian).
