Commit fcd8843c40 breaks old compilers

Commit fcd8843c40 breaks old compilers

[Boris Ostrovsky]

Commit fcd8843c406b46433857ae45e5e9d84b01a7d20b breaks on older
compilers which cannot process initializers for anonymous structures:

+const nfs4_stateid invalid_stateid = {
+       {
+               .seqid = cpu_to_be32(0xffffffffU),
+               .other = { 0 },
+       },
+       .type = NFS4_INVALID_STATEID_TYPE,
+};


/home/build/linux-linus/fs/nfs/nfs4state.c:74: error: unknown field
‘seqid’ specified in initializer
/home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: missing braces
around initializer
/home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: (near
initialization for ‘invalid_stateid.<anonymous>.data’)
/home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: overflow in
implicit constant conversion
/home/build/linux-linus/fs/nfs/nfs4state.c:75: error: unknown field
‘other’ specified in initializer
/home/build/linux-linus/fs/nfs/nfs4state.c:75: error: extra brace group
at end of initializer
/home/build/linux-linus/fs/nfs/nfs4state.c:75: error: (near
initialization for ‘invalid_stateid.<anonymous>’)
/home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: excess elements
in union initializer
/home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: (near
initialization for ‘invalid_stateid.<anonymous>’)
make[4]: *** [fs/nfs/nfs4state.o] Error 1
make[3]: *** [fs/nfs] Error 2


FC-64 <build@build-mk2:~> gcc --version
gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)


A similar bug was fixed by e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91 but
I don't think the same approach can work here.


-boris

Re: Commit fcd8843c40 breaks old compilers

[Trond Myklebust]

On Sat, 2017-11-18 at 12:19 -0500, Boris Ostrovsky wrote:
> Commit fcd8843c406b46433857ae45e5e9d84b01a7d20b breaks on older
> compilers which cannot process initializers for anonymous structures:
> 
> +const nfs4_stateid invalid_stateid = {
> +       {
> +               .seqid = cpu_to_be32(0xffffffffU),
> +               .other = { 0 },
> +       },
> +       .type = NFS4_INVALID_STATEID_TYPE,
> +};
> 
> 
> /home/build/linux-linus/fs/nfs/nfs4state.c:74: error: unknown field
> ‘seqid’ specified in initializer
> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: missing
> braces
> around initializer
> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: (near
> initialization for ‘invalid_stateid.<anonymous>.data’)
> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: overflow in
> implicit constant conversion
> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: unknown field
> ‘other’ specified in initializer
> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: extra brace
> group
> at end of initializer
> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: (near
> initialization for ‘invalid_stateid.<anonymous>’)
> /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: excess
> elements
> in union initializer
> /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: (near
> initialization for ‘invalid_stateid.<anonymous>’)
> make[4]: *** [fs/nfs/nfs4state.o] Error 1
> make[3]: *** [fs/nfs] Error 2
> 
> 
> FC-64 <build@build-mk2:~> gcc --version
> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> 
> 
> A similar bug was fixed by e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91
> but
> I don't think the same approach can work here.


I don't have any setups with gcc 4.4.4. What is it expecting here? Is
it expecting an extra set of braces due to the anonymous "struct"?

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: Commit fcd8843c40 breaks old compilers

[Boris Ostrovsky]

On 11/18/2017 12:39 PM, Trond Myklebust wrote:
> On Sat, 2017-11-18 at 12:19 -0500, Boris Ostrovsky wrote:
>> Commit fcd8843c406b46433857ae45e5e9d84b01a7d20b breaks on older
>> compilers which cannot process initializers for anonymous structures:
>>
>> +const nfs4_stateid invalid_stateid = {
>> +       {
>> +               .seqid = cpu_to_be32(0xffffffffU),
>> +               .other = { 0 },
>> +       },
>> +       .type = NFS4_INVALID_STATEID_TYPE,
>> +};
>>
>>
>> /home/build/linux-linus/fs/nfs/nfs4state.c:74: error: unknown field
>> ‘seqid’ specified in initializer
>> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: missing
>> braces
>> around initializer
>> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: (near
>> initialization for ‘invalid_stateid.<anonymous>.data’)
>> /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: overflow in
>> implicit constant conversion
>> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: unknown field
>> ‘other’ specified in initializer
>> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: extra brace
>> group
>> at end of initializer
>> /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: (near
>> initialization for ‘invalid_stateid.<anonymous>’)
>> /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: excess
>> elements
>> in union initializer
>> /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: (near
>> initialization for ‘invalid_stateid.<anonymous>’)
>> make[4]: *** [fs/nfs/nfs4state.o] Error 1
>> make[3]: *** [fs/nfs] Error 2
>>
>>
>> FC-64 <build@build-mk2:~> gcc --version
>> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
>>
>>
>> A similar bug was fixed by e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91
>> but
>> I don't think the same approach can work here.
> 
> 
> I don't have any setups with gcc 4.4.4. What is it expecting here? Is
> it expecting an extra set of braces due to the anonymous "struct"?
> 

No, that won't work (at least I couldn't get it to work) because the 
solution from e0714ec4f9e assumes that the anonymous struct is the first 
one in the enveloping struct.

It worked only if I (this is a small C program with equivalent structs):

struct nfs4_stateid_struct {
         union {
                 //char data[4];
                 struct {
                         unsigned seqid;
                         char other[6];
                 } __attribute__ ((packed));
		char data[4];
         };
and then

const nfs4_stateid invalid_stateid = {
         {
                   {.seqid = 0xffffffffU,
                   .other = { 0 } },
         },
         .type = NFS4_INVALID_STATEID_TYPE,
};

If I keep data[4] where it is now I get compiler error

an.c:35:20: error: field name not in record or union initializer
                    {.seqid = 0xffffffffU,
                     ^
an.c:35:20: note: (near initialization for 
'invalid_stateid.<anonymous>.data')
an.c:35:29: warning: overflow in implicit constant conversion [-Woverflow]
                    {.seqid = 0xffffffffU,
                              ^~~~~~~~~~~
an.c:36:19: error: field name not in record or union initializer
                    .other = { 0 } },
                    ^
an.c:36:19: note: (near initialization for 
'invalid_stateid.<anonymous>.data')
an.c:36:19: warning: braces around scalar initializer
an.c:36:19: note: (near initialization for 
'invalid_stateid.<anonymous>.data[1]')

I don't know if you want to change public header file just to get around 
this problem.


-boris

Re: Commit fcd8843c40 breaks old compilers

[Trond Myklebust]

On Sat, 2017-11-18 at 13:07 -0500, Boris Ostrovsky wrote:
> 
> On 11/18/2017 12:39 PM, Trond Myklebust wrote:
> > On Sat, 2017-11-18 at 12:19 -0500, Boris Ostrovsky wrote:
> > > Commit fcd8843c406b46433857ae45e5e9d84b01a7d20b breaks on older
> > > compilers which cannot process initializers for anonymous
> > > structures:
> > > 
> > > +const nfs4_stateid invalid_stateid = {
> > > +       {
> > > +               .seqid = cpu_to_be32(0xffffffffU),
> > > +               .other = { 0 },
> > > +       },
> > > +       .type = NFS4_INVALID_STATEID_TYPE,
> > > +};
> > > 
> > > 
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:74: error: unknown
> > > field
> > > ‘seqid’ specified in initializer
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: missing
> > > braces
> > > around initializer
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: (near
> > > initialization for ‘invalid_stateid.<anonymous>.data’)
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:74: warning: overflow
> > > in
> > > implicit constant conversion
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: unknown
> > > field
> > > ‘other’ specified in initializer
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: extra brace
> > > group
> > > at end of initializer
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:75: error: (near
> > > initialization for ‘invalid_stateid.<anonymous>’)
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: excess
> > > elements
> > > in union initializer
> > > /home/build/linux-linus/fs/nfs/nfs4state.c:75: warning: (near
> > > initialization for ‘invalid_stateid.<anonymous>’)
> > > make[4]: *** [fs/nfs/nfs4state.o] Error 1
> > > make[3]: *** [fs/nfs] Error 2
> > > 
> > > 
> > > FC-64 <build@build-mk2:~> gcc --version
> > > gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> > > 
> > > 
> > > A similar bug was fixed by
> > > e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91
> > > but
> > > I don't think the same approach can work here.
> > 
> > 
> > I don't have any setups with gcc 4.4.4. What is it expecting here?
> > Is
> > it expecting an extra set of braces due to the anonymous "struct"?
> > 
> 
> No, that won't work (at least I couldn't get it to work) because the 
> solution from e0714ec4f9e assumes that the anonymous struct is the
> first 
> one in the enveloping struct.
> 
> It worked only if I (this is a small C program with equivalent
> structs):
> 
> struct nfs4_stateid_struct {
>          union {
>                  //char data[4];
>                  struct {
>                          unsigned seqid;
>                          char other[6];
>                  } __attribute__ ((packed));
> 		char data[4];
>          };
> and then
> 
> const nfs4_stateid invalid_stateid = {
>          {
>                    {.seqid = 0xffffffffU,
>                    .other = { 0 } },
>          },
>          .type = NFS4_INVALID_STATEID_TYPE,
> };
> 
> If I keep data[4] where it is now I get compiler error
> 
> an.c:35:20: error: field name not in record or union initializer
>                     {.seqid = 0xffffffffU,
>                      ^
> an.c:35:20: note: (near initialization for 
> 'invalid_stateid.<anonymous>.data')
> an.c:35:29: warning: overflow in implicit constant conversion [-
> Woverflow]
>                     {.seqid = 0xffffffffU,
>                               ^~~~~~~~~~~
> an.c:36:19: error: field name not in record or union initializer
>                     .other = { 0 } },
>                     ^
> an.c:36:19: note: (near initialization for 
> 'invalid_stateid.<anonymous>.data')
> an.c:36:19: warning: braces around scalar initializer
> an.c:36:19: note: (near initialization for 
> 'invalid_stateid.<anonymous>.data[1]')
> 
> I don't know if you want to change public header file just to get
> around 
> this problem.

Sigh.... OK, how about something like the following then:

{ .data = { 0xff, 0xff, 0xff, 0xff, 0 }, }

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: Commit fcd8843c40 breaks old compilers

[Boris Ostrovsky]

On 11/18/2017 01:12 PM, Trond Myklebust wrote:

> 
> Sigh.... OK, how about something like the following then:
> 
> { .data = { 0xff, 0xff, 0xff, 0xff, 0 }, }
> 


Yes, this does build.


diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 54fd56d..daa6085 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -71,8 +71,7 @@
  };
  const nfs4_stateid invalid_stateid = {
         {
-               .seqid = cpu_to_be32(0xffffffffU),
-               .other = { 0 },
+               .data = { 0xff, 0xff, 0xff, 0xff, 0 },
         },
         .type = NFS4_INVALID_STATEID_TYPE,
  };


-boris

[PATCH] NFSv4: Ensure gcc 4.4.4 can compile initialiser for "invalid_stateid"

[Trond Myklebust]

gcc 4.4.4 is too old to have full C11 anonymous union support, so
the current initialiser fails to compile.

Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 980462d577ca..231b5ea2464a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -71,8 +71,8 @@ const nfs4_stateid zero_stateid = {
 };
 const nfs4_stateid invalid_stateid = {
 	{
-		.seqid = cpu_to_be32(0xffffffffU),
-		.other = { 0 },
+		/* Funky initialiser keeps older gcc versions happy */
+		.data = { 0xff, 0xff, 0xff, 0xff, 0 },
 	},
 	.type = NFS4_INVALID_STATEID_TYPE,
 };
-- 
2.14.3

Re: [PATCH] NFSv4: Ensure gcc 4.4.4 can compile initialiser for "invalid_stateid"

[Boris Ostrovsky]

On 11/18/2017 01:50 PM, Trond Myklebust wrote:
> gcc 4.4.4 is too old to have full C11 anonymous union support, so
> the current initialiser fails to compile.
> 
> Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>


(compile-)Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>   fs/nfs/nfs4state.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 980462d577ca..231b5ea2464a 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -71,8 +71,8 @@ const nfs4_stateid zero_stateid = {
>   };
>   const nfs4_stateid invalid_stateid = {
>   	{
> -		.seqid = cpu_to_be32(0xffffffffU),
> -		.other = { 0 },
> +		/* Funky initialiser keeps older gcc versions happy */
> +		.data = { 0xff, 0xff, 0xff, 0xff, 0 },
>   	},
>   	.type = NFS4_INVALID_STATEID_TYPE,
>   };
> 

Re: [PATCH] NFSv4: Ensure gcc 4.4.4 can compile initialiser for "invalid_stateid"

[Geert Uytterhoeven]

On Sat, Nov 18, 2017 at 7:50 PM, Trond Myklebust
<trond.myklebust@primarydata.com> wrote:
> gcc 4.4.4 is too old to have full C11 anonymous union support, so
> the current initialiser fails to compile.

Thanks! Works fine with gcc-4.1.2, too.

> Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: Commit fcd8843c40 breaks old compilers

[Arnd Bergmann]

On Sat, Nov 18, 2017 at 7:07 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
>
> On 11/18/2017 12:39 PM, Trond Myklebust wrote:
>>
>> On Sat, 2017-11-18 at 12:19 -0500, Boris Ostrovsky wrote:
>>>

>>> A similar bug was fixed by e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91
>>> but
>>> I don't think the same approach can work here.
>>
>>
>>
>> I don't have any setups with gcc 4.4.4. What is it expecting here? Is
>> it expecting an extra set of braces due to the anonymous "struct"?

I can reproduce it with gcc-4.5 but not 4.6. We've had similar problems
in the past in other anonymous unions.

> I don't know if you want to change public header file just to get around
> this problem.

I think flipping the two members around should be safe here. It's
not exported to user space, and all other users of that structure
don't care about the order inside of the union.

       Arnd

Re: Commit fcd8843c40 breaks old compilers

[Boris Ostrovsky]

On 11/20/2017 07:52 AM, Arnd Bergmann wrote:
> On Sat, Nov 18, 2017 at 7:07 PM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>>
>> On 11/18/2017 12:39 PM, Trond Myklebust wrote:
>>> On Sat, 2017-11-18 at 12:19 -0500, Boris Ostrovsky wrote:
>>>> A similar bug was fixed by e0714ec4f9efe7b86828b0dcc077fd8f5d8e5e91
>>>> but
>>>> I don't think the same approach can work here.
>>>
>>>
>>> I don't have any setups with gcc 4.4.4. What is it expecting here? Is
>>> it expecting an extra set of braces due to the anonymous "struct"?
> I can reproduce it with gcc-4.5 but not 4.6. We've had similar problems
> in the past in other anonymous unions.

IIRC anonymous struct initializers were added in 4.6.1.

>
>> I don't know if you want to change public header file just to get around
>> this problem.
> I think flipping the two members around should be safe here. It's
> not exported to user space, and all other users of that structure
> don't care about the order inside of the union.

Trond already submitted a fix ---
<20171118185011.4729-1-trond.myklebust@primarydata.com> (sorry, I can't
provide a direct link right now)

-boris