[PATCH] [RFC] bcache: hide variable-sized types from uapi headers

[PATCH] [RFC] bcache: hide variable-sized types from uapi headers

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The headers_check helper complains about a GNU extension in
one of the exported headers:

linux/bcache.h:354:2: warning: field '' with variable sized type 'union jset::(anonymous at ./usr/include/linux/bcache.h:354:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
        BKEY_PADDED(uuid_bucket);
        ^
linux/bcache.h:134:2: note: expanded from macro 'BKEY_PADDED'
        union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }
        ^

We could either try to shut up the warning or remove those parts from
the user-visible section of this header. This does the second,
under the assumption that they are not actually used.

Fixes: 81ab4190ac17 ("bcache: Pull on disk data structures out into a separate header")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Coli, Kent: can you check to see if the hidden parts are used anywhere
from user space, and apply the patch if not?

Any other ideas for addressing this warning?
---
 include/uapi/linux/bcache.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/bcache.h b/include/uapi/linux/bcache.h
index cf7399f03b71..e3e4889aa53e 100644
--- a/include/uapi/linux/bcache.h
+++ b/include/uapi/linux/bcache.h
@@ -23,9 +23,13 @@ static inline void SET_##name(type *k, __u64 v)			\
 struct bkey {
 	__u64	high;
 	__u64	low;
+#ifdef __KERNEL__
+	/* gcc extension not meant for user space */
 	__u64	ptr[];
+#endif
 };
 
+#ifdef __KERNEL__
 #define KEY_FIELD(name, field, offset, size)				\
 	BITMASK(name, struct bkey, field, offset, size)
 
@@ -127,6 +131,8 @@ static inline struct bkey *bkey_idx(const struct bkey *k, unsigned int nr_keys)
 
 	return (struct bkey *) (d + nr_keys);
 }
+#endif
+
 /* Enough for a key with 6 pointers */
 #define BKEY_PAD		8
 
-- 
2.29.2

Re: [PATCH] [RFC] bcache: hide variable-sized types from uapi headers

[Coly Li]

On 9/27/21 8:43 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The headers_check helper complains about a GNU extension in
> one of the exported headers:
>
> linux/bcache.h:354:2: warning: field '' with variable sized type 'union jset::(anonymous at ./usr/include/linux/bcache.h:354:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
>          BKEY_PADDED(uuid_bucket);
>          ^
> linux/bcache.h:134:2: note: expanded from macro 'BKEY_PADDED'
>          union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }
>          ^
>
> We could either try to shut up the warning or remove those parts from
> the user-visible section of this header. This does the second,
> under the assumption that they are not actually used.

Hi Arnd,

Yes, the variable part is necessary for bcache-tools to understand the 
on-disk format. If other program wants to understand the bcache on-disk 
format, IMHO such variable part might be necessary too.

BKEY_PADDED() is a special usage of the variable size array. In this 
case it indicates uuid_bucket is 8 x u64, and 6 x u64 space for ptr[]. 
And the bcache code make sure no more than 6 pointers are used for 
uuid_bucket.

I know BKEY_PADDED() works, but I don't know how to simply remove the 
-Wgnu-variable-sized-type-not-at-end warning.

Maybe Kent may offer some hint ?

Coly Li


>
> Fixes: 81ab4190ac17 ("bcache: Pull on disk data structures out into a separate header")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Coli, Kent: can you check to see if the hidden parts are used anywhere
> from user space, and apply the patch if not?
>
> Any other ideas for addressing this warning?
> ---
>   include/uapi/linux/bcache.h | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/include/uapi/linux/bcache.h b/include/uapi/linux/bcache.h
> index cf7399f03b71..e3e4889aa53e 100644
> --- a/include/uapi/linux/bcache.h
> +++ b/include/uapi/linux/bcache.h
> @@ -23,9 +23,13 @@ static inline void SET_##name(type *k, __u64 v)			\
>   struct bkey {
>   	__u64	high;
>   	__u64	low;
> +#ifdef __KERNEL__
> +	/* gcc extension not meant for user space */
>   	__u64	ptr[];
> +#endif
>   };
>   
> +#ifdef __KERNEL__
>   #define KEY_FIELD(name, field, offset, size)				\
>   	BITMASK(name, struct bkey, field, offset, size)
>   
> @@ -127,6 +131,8 @@ static inline struct bkey *bkey_idx(const struct bkey *k, unsigned int nr_keys)
>   
>   	return (struct bkey *) (d + nr_keys);
>   }
> +#endif
> +
>   /* Enough for a key with 6 pointers */
>   #define BKEY_PAD		8
>   

Re: [PATCH] [RFC] bcache: hide variable-sized types from uapi headers

[Arnd Bergmann]

On Mon, Sep 27, 2021 at 5:22 PM Coly Li <colyli@suse.de> wrote:
>
> On 9/27/21 8:43 PM, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The headers_check helper complains about a GNU extension in
> > one of the exported headers:
> >
> > linux/bcache.h:354:2: warning: field '' with variable sized type 'union jset::(anonymous at ./usr/include/linux/bcache.h:354:2)' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> >          BKEY_PADDED(uuid_bucket);
> >          ^
> > linux/bcache.h:134:2: note: expanded from macro 'BKEY_PADDED'
> >          union { struct bkey key; __u64 key ## _pad[BKEY_PAD]; }
> >          ^
> >
> > We could either try to shut up the warning or remove those parts from
> > the user-visible section of this header. This does the second,
> > under the assumption that they are not actually used.
>
> Hi Arnd,
>
> Yes, the variable part is necessary for bcache-tools to understand the
> on-disk format. If other program wants to understand the bcache on-disk
> format, IMHO such variable part might be necessary too.

Ok, got it.

> BKEY_PADDED() is a special usage of the variable size array. In this
> case it indicates uuid_bucket is 8 x u64, and 6 x u64 space for ptr[].
> And the bcache code make sure no more than 6 pointers are used for
> uuid_bucket.
>
> I know BKEY_PADDED() works, but I don't know how to simply remove the
> -Wgnu-variable-sized-type-not-at-end warning.
>
> Maybe Kent may offer some hint ?

Maybe instead of checking for __KERNEL__, we could check for
__STRICT_ANSI__ being unset. This would hide the definition
from user space applications that don't have this extension, and from
the header check but still allow it in all applications built with gcc
or clang that don't request the strict standard compliance.

Another option is to exclude this header from the header-test
in usr/include/Makefile.

        Arnd