[dpdk-dev] [PATCH] cryptodev: fix compilation error in SUSE 11 SP2

[Adrien Mazarguil]

On Thu, Sep 29, 2016 at 07:30:31PM +0000, De Lara Guarch, Pablo wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > Sent: Tuesday, September 27, 2016 12:45 AM
> > To: De Lara Guarch, Pablo
> > Cc: dev at dpdk.org; Doherty, Declan
> > Subject: Re: [PATCH] cryptodev: fix compilation error in SUSE 11 SP2
> > 
> > On Mon, Sep 26, 2016 at 10:50:35PM +0100, Pablo de Lara wrote:
> > > This commit fixes following build error, which happens in SUSE 11 SP2,
> > > with gcc 4.5.1:
> > >
> > > In file included from lib/librte_cryptodev/rte_cryptodev.c:71:0:
> > > lib/librte_cryptodev/rte_cryptodev_pmd.h:76:7:
> > > error: flexible array member in otherwise empty struct
> > >
> > > Fixes: 347a1e037fd3 ("lib: use C99 syntax for zero-size arrays")
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch at intel.com>
> > 
> > Hmm, this error message does not seem related to your patch. Assuming a
> > similar error is caused by the original code, I think there is a more
> > important issue as the struct should not be empty. Can you check the
> > error?
> 
> Well, I don't really understand what is the difference between array[] and array[0],
> I thought both were the same, but some compilers only accept the latter.

Before array[] got standardized by C99, a common trick was to use array[0],
in a sense they are similar except for this one case: a struct with a single
array[] field is explicitly not allowed in C99 since it causes the structure
to be empty (this syntax only provides an alignment constraint for what
follows in case padding is required), no such problem with array[0] which
although nonstandard, is an accepted behavior, sizeof(struct foo) may yield
0 without complaint.

> In any case, the struct will not be empty, as there are other fields, that are not variable sized.
> 
> I saw that in your patch you made these two changes (among others):
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index affbdec..1e30a19 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -759,7 +759,7 @@ struct rte_cryptodev_sym_session {
>         } __rte_aligned(8);
>         /**< Public symmetric session details */
> 
> -       char _private[0];
> +       char _private[];
>         /**< Private session material */
>  };
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> index 7d049ea..42e7b79 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> @@ -71,7 +71,7 @@ struct rte_cryptodev_session {
>                 struct rte_mempool *mp;
>         } __rte_aligned(8);
> 
> -       char _private[0];
> +       __extension__ char _private[0];
>  };
> 
> So I would expect the same change in both, as they are almost identical,
> but you took different approaches (do you know why? I would like to know :))

Yes, this was done to address the exact same error (probably with the same
old GCC version (4.4.7 perhaps?)), hence my surprise to see it fixed once
again according to your commit log, I think your only mistake was to paste
the error message for the wrong header in there (rte_cryptodev_pmd.h instead
of rte_cryptodev.h), nothing wrong with your patch besides this.

> Basically, I noticed that gcc 4.5 doesn't complain when using your second approach,
> that's why I changed it.

For the record GCC wrongly thinks the structure is empty because a unnamed
struct field is declared inside. Before C11 such declarations only created a
new type that did not occupy any space and not an actual field, hence why it
complains when faced with [] instead of the well-behaved [0].

In this particular case it's a parsing error fixed in subsequent GCC
versions, the unnamed struct actually uses some space otherwise it would
have crashed during non-regression testing (right?)

-- 
Adrien Mazarguil
6WIND