selftest: net: fix variable sized type not at the end of struct warnings

[PATCH] selftest: net: fix variable sized type not at the end of struct warnings

Posted by Ankit Khushwaha 3 months, 1 week ago

Some network selftests defined variable-sized types defined at the end of
struct causing -Wgnu-variable-sized-type-not-at-end warning.

warning:
timestamping.c:285:18: warning: field 'cm' with variable sized type 
'struct cmsghdr' not at the end of a struct or class is a GNU 
extension [-Wgnu-variable-sized-type-not-at-end]
  285 |                 struct cmsghdr cm;
      |                                ^

ipsec.c:835:5: warning: field 'u' with variable sized type 'union 
(unnamed union at ipsec.c:831:3)' not at the end of a struct or class 
is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
  835 |                 } u;
      |                   ^

This patch move these field at the end of struct to fix these warnings.

Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>
---
 tools/testing/selftests/net/ipsec.c        | 2 +-
 tools/testing/selftests/net/timestamping.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/ipsec.c b/tools/testing/selftests/net/ipsec.c
index 0ccf484b1d9d..36083c8f884f 100644
--- a/tools/testing/selftests/net/ipsec.c
+++ b/tools/testing/selftests/net/ipsec.c
@@ -828,12 +828,12 @@ static int xfrm_state_pack_algo(struct nlmsghdr *nh, size_t req_sz,
 		struct xfrm_desc *desc)
 {
 	struct {
+		char buf[XFRM_ALGO_KEY_BUF_SIZE];
 		union {
 			struct xfrm_algo	alg;
 			struct xfrm_algo_aead	aead;
 			struct xfrm_algo_auth	auth;
 		} u;
-		char buf[XFRM_ALGO_KEY_BUF_SIZE];
 	} alg = {};
 	size_t alen, elen, clen, aelen;
 	unsigned short type;
diff --git a/tools/testing/selftests/net/timestamping.c b/tools/testing/selftests/net/timestamping.c
index 044bc0e9ed81..ad2be2143698 100644
--- a/tools/testing/selftests/net/timestamping.c
+++ b/tools/testing/selftests/net/timestamping.c
@@ -282,8 +282,8 @@ static void recvpacket(int sock, int recvmsg_flags,
 	struct iovec entry;
 	struct sockaddr_in from_addr;
 	struct {
-		struct cmsghdr cm;
 		char control[512];
+		struct cmsghdr cm;
 	} control;
 	int res;
 
-- 
2.51.0

Re: [PATCH] selftest: net: fix variable sized type not at the end of struct warnings

Posted by Simon Horman 3 months, 1 week ago

On Mon, Oct 27, 2025 at 10:38:56AM +0530, Ankit Khushwaha wrote:
> Some network selftests defined variable-sized types defined at the end of
> struct causing -Wgnu-variable-sized-type-not-at-end warning.
> 
> warning:
> timestamping.c:285:18: warning: field 'cm' with variable sized type 
> 'struct cmsghdr' not at the end of a struct or class is a GNU 
> extension [-Wgnu-variable-sized-type-not-at-end]
>   285 |                 struct cmsghdr cm;
>       |                                ^
> 
> ipsec.c:835:5: warning: field 'u' with variable sized type 'union 
> (unnamed union at ipsec.c:831:3)' not at the end of a struct or class 
> is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
>   835 |                 } u;
>       |                   ^
> 
> This patch move these field at the end of struct to fix these warnings.
> 
> Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>

Hi Ankit,

I don't believe this change is correct.

I think that the intention of the code is the char arrays (buf and control)
provide the buffer space for the variable-length trailing field
of the preceding structure. Where we basically have a header followed
by data. But your patch would place the before the header.

-- 
pw-bot: cr

Re: [PATCH] selftest: net: fix variable sized type not at the end of struct warnings

Posted by Ankit Khushwaha 3 months ago

On Tue, Oct 28, 2025 at 05:23:12PM +0000, Simon Horman wrote:
> On Mon, Oct 27, 2025 at 10:38:56AM +0530, Ankit Khushwaha wrote:
> > Some network selftests defined variable-sized types defined at the end of
> > struct causing -Wgnu-variable-sized-type-not-at-end warning.
> > 
> > warning:
> > timestamping.c:285:18: warning: field 'cm' with variable sized type 
> > 'struct cmsghdr' not at the end of a struct or class is a GNU 
> > extension [-Wgnu-variable-sized-type-not-at-end]
> >   285 |                 struct cmsghdr cm;
> >       |                                ^
> > 
> > ipsec.c:835:5: warning: field 'u' with variable sized type 'union 
> > (unnamed union at ipsec.c:831:3)' not at the end of a struct or class 
> > is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> >   835 |                 } u;
> >       |                   ^
> > 
> > This patch move these field at the end of struct to fix these warnings.
> > 
> > Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>
> 
> Hi Ankit,
> 
> I don't believe this change is correct.
> 
> I think that the intention of the code is the char arrays (buf and control)
> provide the buffer space for the variable-length trailing field
> of the preceding structure. Where we basically have a header followed
> by data. But your patch would place the before the header.
>
Hi Simon,
So if buf and control providing the buffer space, then i think it is
better to suppress `-Wgnu-variable-sized-type-not-at-end` warning 
within this block of code.

	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Wgnu-variable-sized-type-not-at-end"

	struct {
		union {
			struct xfrm_algo        alg;
			struct xfrm_algo_aead   aead;
			struct xfrm_algo_auth   auth;
		} u;
		char buf[XFRM_ALGO_KEY_BUF_SIZE];
	} alg = {};

	#pragma GCC diagnostic pop

I think this would be fine.

Thanks
-- Ankit

Re: [PATCH] selftest: net: fix variable sized type not at the end of struct warnings

Posted by Simon Horman 2 months, 3 weeks ago

On Sat, Nov 08, 2025 at 10:59:59PM +0530, Ankit Khushwaha wrote:
> On Tue, Oct 28, 2025 at 05:23:12PM +0000, Simon Horman wrote:
> > On Mon, Oct 27, 2025 at 10:38:56AM +0530, Ankit Khushwaha wrote:
> > > Some network selftests defined variable-sized types defined at the end of
> > > struct causing -Wgnu-variable-sized-type-not-at-end warning.
> > > 
> > > warning:
> > > timestamping.c:285:18: warning: field 'cm' with variable sized type 
> > > 'struct cmsghdr' not at the end of a struct or class is a GNU 
> > > extension [-Wgnu-variable-sized-type-not-at-end]
> > >   285 |                 struct cmsghdr cm;
> > >       |                                ^
> > > 
> > > ipsec.c:835:5: warning: field 'u' with variable sized type 'union 
> > > (unnamed union at ipsec.c:831:3)' not at the end of a struct or class 
> > > is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> > >   835 |                 } u;
> > >       |                   ^
> > > 
> > > This patch move these field at the end of struct to fix these warnings.
> > > 
> > > Signed-off-by: Ankit Khushwaha <ankitkhushwaha.linux@gmail.com>
> > 
> > Hi Ankit,
> > 
> > I don't believe this change is correct.
> > 
> > I think that the intention of the code is the char arrays (buf and control)
> > provide the buffer space for the variable-length trailing field
> > of the preceding structure. Where we basically have a header followed
> > by data. But your patch would place the before the header.
> >
> Hi Simon,
> So if buf and control providing the buffer space, then i think it is
> better to suppress `-Wgnu-variable-sized-type-not-at-end` warning 
> within this block of code.
> 
> 	#pragma GCC diagnostic push
> 	#pragma GCC diagnostic ignored "-Wgnu-variable-sized-type-not-at-end"

I'm unsure of the attitude towards using #pragma like this in kernel code,
but certainly it would be a new one for me.

> 
> 	struct {
> 		union {
> 			struct xfrm_algo        alg;
> 			struct xfrm_algo_aead   aead;
> 			struct xfrm_algo_auth   auth;
> 		} u;
> 		char buf[XFRM_ALGO_KEY_BUF_SIZE];
> 	} alg = {};
> 
> 	#pragma GCC diagnostic pop
> 
> I think this would be fine.

In my view, the most promising approach I am aware of is using
TRAILING_OVERLAP(), as illustrated in [1]. However, that
approach was recently rejected, so I guess that means
it doesn't have much promise after all.

[1] https://lore.kernel.org/netdev/aPdx4iPK4-KIhjFq@kspp/