| Message ID | 20240213234212.3766256-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | In Next |
| Commit | bcc5b5edef92ba1f79d04d13d92444796cf41704 |
| Headers | show |
| Series | stddef: Allow attributes to be used when creating flex arrays | expand |
Hi, Kees I was looking to apply the __counted_by to the drivers/net/can subtree, and a research on the DECLARE_FLEX_ARRAY brought me to this patch. I could not find it in any tree (tried Linus's tree and linux-next), so I am not sure what is the status here (sorry if it was upstreamed and if I just missed it). While at it, and with several months of delays, here is my feedback. On Tue, 13 Feb 2024 at 15:42:10, Kees Cook <keescook@chromium.org> wrote: > With the coming support for the __counted_by struct member attribute, > we will need a way to add such annotations to the places where > DECLARE_FLEX_ARRAY() is used. Add an optional 3rd argument that can be > used for including attributes in the flexible array definition. > > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Keith Packard <keithp@keithp.com> > Cc: Miguel Ojeda <ojeda@kernel.org> > Cc: Alexey Dobriyan <adobriyan@gmail.com> > Cc: Dmitry Antipov <dmantipov@yandex.ru> > Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > include/linux/stddef.h | 6 +++--- > include/uapi/linux/stddef.h | 10 +++++----- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/include/linux/stddef.h b/include/linux/stddef.h > index 929d67710cc5..176bfe8c0bd7 100644 > --- a/include/linux/stddef.h > +++ b/include/linux/stddef.h > @@ -82,15 +82,15 @@ enum { > > /** > * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union > - * Nitpick: this line removal is not related to the patch and the other documentation blocks in include/linux/stddef.h also have this empty line. For consistency, better to keep. > * @TYPE: The type of each flexible array element > * @NAME: The name of the flexible array member > + * @...: The list of member attributes to apply (optional) > * > * In order to have a flexible array member in a union or alone in a > * struct, it needs to be wrapped in an anonymous struct with at least 1 > * named member, but that member can be empty. > */ > -#define DECLARE_FLEX_ARRAY(TYPE, NAME) \ > - __DECLARE_FLEX_ARRAY(TYPE, NAME) > +#define DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ > + __DECLARE_FLEX_ARRAY(TYPE, NAME, __VA_ARGS__) > > #endif > diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h > index 2ec6f35cda32..028aeec3d7f1 100644 > --- a/include/uapi/linux/stddef.h > +++ b/include/uapi/linux/stddef.h > @@ -31,23 +31,23 @@ > > #ifdef __cplusplus > /* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */ > -#define __DECLARE_FLEX_ARRAY(T, member) \ > - T member[0] > +#define __DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ > + TYPE NAME[0] __VA_ARGS__ > #else > /** > * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union > - * Same as above: no need to remove. > * @TYPE: The type of each flexible array element > * @NAME: The name of the flexible array member > + * @...: The list of member attributes to apply (optional) > * > * In order to have a flexible array member in a union or alone in a > * struct, it needs to be wrapped in an anonymous struct with at least 1 > * named member, but that member can be empty. > */ > -#define __DECLARE_FLEX_ARRAY(TYPE, NAME) \ > +#define __DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ > struct { \ > struct { } __empty_ ## NAME; \ > - TYPE NAME[]; \ > + TYPE NAME[] __VA_ARGS__; \ > } > #endif How does this work? If I take this example: struct foo { size_t union_size; union { struct bar; DECLARE_FLEX_ARRAY(u8, raw, __counted_by(union_size)); }; }; it will expand to: struct foo { size_t union_size; union { struct bar; struct { struct { } __empty_raw; u8 raw[] __counted_by(union_size); }; }; }; right? Looking at clang documentation: The count field member must be within the same non-anonymous, enclosing struct as the flexible array member. Ref: https://clang.llvm.org/docs/AttributeReference.html#counted-by Here, the union_size and the flexible array member are in different structures (struct foo and anonymous structure). It seems to me that the prerequisites are not met. Am I missing something? Yours sincerely, Vincent Mailhol
diff --git a/include/linux/stddef.h b/include/linux/stddef.h index 929d67710cc5..176bfe8c0bd7 100644 --- a/include/linux/stddef.h +++ b/include/linux/stddef.h @@ -82,15 +82,15 @@ enum { /** * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union - * * @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member + * @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1 * named member, but that member can be empty. */ -#define DECLARE_FLEX_ARRAY(TYPE, NAME) \ - __DECLARE_FLEX_ARRAY(TYPE, NAME) +#define DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ + __DECLARE_FLEX_ARRAY(TYPE, NAME, __VA_ARGS__) #endif diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h index 2ec6f35cda32..028aeec3d7f1 100644 --- a/include/uapi/linux/stddef.h +++ b/include/uapi/linux/stddef.h @@ -31,23 +31,23 @@ #ifdef __cplusplus /* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */ -#define __DECLARE_FLEX_ARRAY(T, member) \ - T member[0] +#define __DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ + TYPE NAME[0] __VA_ARGS__ #else /** * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union - * * @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member + * @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1 * named member, but that member can be empty. */ -#define __DECLARE_FLEX_ARRAY(TYPE, NAME) \ +#define __DECLARE_FLEX_ARRAY(TYPE, NAME, ...) \ struct { \ struct { } __empty_ ## NAME; \ - TYPE NAME[]; \ + TYPE NAME[] __VA_ARGS__; \ } #endif