On Wed, 2021-07-28 at 14:59 -0700, Kees Cook wrote:
On Wed, Jul 28, 2021 at 12:54:18PM +0200, Rasmus Villemoes wrote:
On 27/07/2021 22.57, Kees Cook wrote:
In order to have a regular programmatic way to describe a struct region that can be used for references and sizing, can be examined for bounds checking, avoids forcing the use of intermediate identifiers, and avoids polluting the global namespace, introduce the struct_group() macro. This macro wraps the member declarations to create an anonymous union of an anonymous struct (no intermediate name) and a named struct (for references and sizing):
struct foo { int one; struct_group(thing, int two, int three, ); int four; };
That example won't compile, the commas after two and three should be semicolons.
Oops, yes, thanks. This is why I shouldn't write code that doesn't first go through a compiler. ;)
And your implementation relies on MEMBERS not containing any comma tokens, but as
int a, b, c, d;
is a valid way to declare multiple members, consider making MEMBERS variadic
#define struct_group(NAME, MEMBERS...)
to have it slurp up every subsequent argument and make that work.
Ah! Perfect, thank you. I totally forgot I could do it that way.
This is great Kees. It just so happens it would clean-up what we are already doing in drivers/cxl/cxl.h for anonymous + named register block pointers. However in the cxl case it also needs the named structure to be typed. Any appetite for a typed version of this?
Here is a rough idea of the cleanup it would induce in drivers/cxl/:
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 53927f9fa77e..a2308c995654 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -75,52 +75,19 @@ static inline int cxl_hdm_decoder_count(u32 cap_hdr) #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18 #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
-#define CXL_COMPONENT_REGS() \ - void __iomem *hdm_decoder - -#define CXL_DEVICE_REGS() \ - void __iomem *status; \ - void __iomem *mbox; \ - void __iomem *memdev - -/* See note for 'struct cxl_regs' for the rationale of this organization */ /* - * CXL_COMPONENT_REGS - Common set of CXL Component register block base pointers * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure - */ -struct cxl_component_regs { - CXL_COMPONENT_REGS(); -}; - -/* See note for 'struct cxl_regs' for the rationale of this organization */ -/* - * CXL_DEVICE_REGS - Common set of CXL Device register block base pointers * @status: CXL 2.0 8.2.8.3 Device Status Registers * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers */ -struct cxl_device_regs { - CXL_DEVICE_REGS(); -}; - -/* - * Note, the anonymous union organization allows for per - * register-block-type helper routines, without requiring block-type - * agnostic code to include the prefix. - */ struct cxl_regs { - union { - struct { - CXL_COMPONENT_REGS(); - }; - struct cxl_component_regs component; - }; - union { - struct { - CXL_DEVICE_REGS(); - }; - struct cxl_device_regs device_regs; - }; + struct_group_typed(cxl_component_regs, component, + void __iomem *hdm_decoder; + ); + struct_group_typed(cxl_device_regs, device_regs, + void __iomem *status, *mbox, *memdev; + ); };
struct cxl_reg_map { diff --git a/include/linux/stddef.h b/include/linux/stddef.h index cf7f866944f9..84b7de24ffb5 100644 --- a/include/linux/stddef.h +++ b/include/linux/stddef.h @@ -49,12 +49,18 @@ enum { * @ATTRS: Any struct attributes (normally empty) * @MEMBERS: The member declarations for the mirrored structs */ -#define struct_group_attr(NAME, ATTRS, MEMBERS) \ +#define struct_group_attr(NAME, ATTRS, MEMBERS...) \ union { \ struct { MEMBERS } ATTRS; \ struct { MEMBERS } ATTRS NAME; \ }
+#define struct_group_attr_typed(TYPE, NAME, ATTRS, MEMBERS...) \ + union { \ + struct { MEMBERS } ATTRS; \ + struct TYPE { MEMBERS } ATTRS NAME; \ + } + /** * struct_group(NAME, MEMBERS) * @@ -67,7 +73,10 @@ enum { * @NAME: The name of the mirrored sub-struct * @MEMBERS: The member declarations for the mirrored structs */ -#define struct_group(NAME, MEMBERS) \ +#define struct_group(NAME, MEMBERS...) \ struct_group_attr(NAME, /* no attrs */, MEMBERS)
+#define struct_group_typed(TYPE, NAME, MEMBERS...) \ + struct_group_attr_typed(TYPE, NAME, /* no attrs */, MEMBERS) + #endif