[dpdk-dev] KNI: fix compilation warning 'missing-field-initializers'

  Fix for compilation warning 'missing-field-initializers' for some
GCC and clang versions introduced in commit 0c6bc8e

Signed-off-by: Marc Sune <marc.sune@bisdn.de>
---
 lib/librte_kni/rte_kni.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
  

Liu,

Can you confirm that this patch fixes the issue?

Thanks
marc

On 22/10/14 09:10, Marc Sune wrote:
> Fix for compilation warning 'missing-field-initializers' for some
> GCC and clang versions introduced in commit 0c6bc8e
>
> Signed-off-by: Marc Sune <marc.sune@bisdn.de>
> ---
>   lib/librte_kni/rte_kni.c |    9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index f64a0a8..de29b99 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -131,7 +131,14 @@ static void kni_free_mbufs(struct rte_kni *kni);
>   static void kni_allocate_mbufs(struct rte_kni *kni);
>   
>   static volatile int kni_fd = -1;
> -static struct rte_kni_memzone_pool kni_memzone_pool = {0};
> +static struct rte_kni_memzone_pool kni_memzone_pool = {
> +	.initialized = 0,
> +	.max_ifaces = 0,
> +	.slots = 0,
> +	.mutex =  RTE_SPINLOCK_INITIALIZER,
> +	.free = NULL,
> +	.free_tail = NULL
> +};
>   
>   static const struct rte_memzone *
>   kni_memzone_reserve(const char *name, size_t len, int socket_id,
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Marc Sune
> Sent: Wednesday, October 22, 2014 3:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] KNI: fix compilation warning 'missing-field-
> initializers'
> 
> Fix for compilation warning 'missing-field-initializers' for some GCC and clang
> versions introduced in commit 0c6bc8e
> 
> Signed-off-by: Marc Sune <marc.sune@bisdn.de>
> ---
>  lib/librte_kni/rte_kni.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index
> f64a0a8..de29b99 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -131,7 +131,14 @@ static void kni_free_mbufs(struct rte_kni *kni);
> static void kni_allocate_mbufs(struct rte_kni *kni);
> 
>  static volatile int kni_fd = -1;
> -static struct rte_kni_memzone_pool kni_memzone_pool = {0};
> +static struct rte_kni_memzone_pool kni_memzone_pool = {
> +	.initialized = 0,
> +	.max_ifaces = 0,
> +	.slots = 0,
> +	.mutex =  RTE_SPINLOCK_INITIALIZER,
> +	.free = NULL,
> +	.free_tail = NULL
> +};
> 
>  static const struct rte_memzone *
>  kni_memzone_reserve(const char *name, size_t len, int socket_id,
> --
> 1.7.10.4

Acked-by: Jijiang Liu<Jijiang.liu@intel.com>
  

2014-10-22 09:10, Marc Sune:
> Fix for compilation warning 'missing-field-initializers' for some
> GCC and clang versions introduced in commit 0c6bc8e
> 
> Signed-off-by: Marc Sune <marc.sune@bisdn.de>

It's not needed to initialize all fields.
This should be sufficient:
+static struct rte_kni_memzone_pool kni_memzone_pool = {.initialized = 0};
  

The mutex needs to be initialized to RTE_SPINLOCK_INITIALIZER(0) too, or 
move the initialization of the mutex to rte_kni_init().

I can prepare a second patch with one or the other option, if you want.

marc

On 22/10/14 10:37, Thomas Monjalon wrote:
> 2014-10-22 09:10, Marc Sune:
>> Fix for compilation warning 'missing-field-initializers' for some
>> GCC and clang versions introduced in commit 0c6bc8e
>>
>> Signed-off-by: Marc Sune <marc.sune@bisdn.de>
> It's not needed to initialize all fields.
> This should be sufficient:
> +static struct rte_kni_memzone_pool kni_memzone_pool = {.initialized = 0};
>
  

2014-10-22 10:42, Marc Sune:
> The mutex needs to be initialized to RTE_SPINLOCK_INITIALIZER(0) too, or 
> move the initialization of the mutex to rte_kni_init().

RTE_SPINLOCK_INITIALIZER is { 0 }
By initializing one field, all other fields are set to 0, so spinlock also.
Just choose one field and it's OK.
It should be tested with ICC also but I think it's OK.

> I can prepare a second patch with one or the other option, if you want.

Yes please.

> On 22/10/14 10:37, Thomas Monjalon wrote:
> > 2014-10-22 09:10, Marc Sune:
> >> Fix for compilation warning 'missing-field-initializers' for some
> >> GCC and clang versions introduced in commit 0c6bc8e
> >>
> >> Signed-off-by: Marc Sune <marc.sune@bisdn.de>
> > It's not needed to initialize all fields.
> > This should be sufficient:
> > +static struct rte_kni_memzone_pool kni_memzone_pool = {.initialized = 0};

Please Marc, don't top post.
Thanks
  

On 22/10/14 10:50, Thomas Monjalon wrote:
> 2014-10-22 10:42, Marc Sune:
>> The mutex needs to be initialized to RTE_SPINLOCK_INITIALIZER(0) too, or
>> move the initialization of the mutex to rte_kni_init().
> RTE_SPINLOCK_INITIALIZER is { 0 }
> By initializing one field, all other fields are set to 0, so spinlock also.
> Just choose one field and it's OK.
> It should be tested with ICC also but I think it's OK.

Seems that you are right, at least for C99:

    C99 Standard 6.7.8.21

         If there are fewer initializers in a brace-enclosed list than
    there are elements or members of an aggregate, or fewer characters
    in a string literal used to initialize an array of known size than
    there are elements in the array, the remainder of the aggregate
    shall be initialized implicitly the same as objects that have static
    storage duration.


I am not sure if there can be problems with other C dialects (e.g. C11), 
I don't have the std here. So to prevent any problem with them (could 
produce a dead-lock during first rte_kni_alloc() that could be difficult 
to troubleshoot), I would still explicitly initialize the mutex, in one 
or the other way.

Just tell me if you agree and which one you prefer.

I don't have an ICC license. I am always trying it with GCC and clang.

Marc
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Marc Sune
> Sent: Wednesday, October 22, 2014 10:50 AM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] KNI: fix compilation warning 'missing-field-
> initializers'
> 
> On 22/10/14 10:50, Thomas Monjalon wrote:
> > 2014-10-22 10:42, Marc Sune:
> >> The mutex needs to be initialized to RTE_SPINLOCK_INITIALIZER(0) too, or
> >> move the initialization of the mutex to rte_kni_init().
> > RTE_SPINLOCK_INITIALIZER is { 0 }
> > By initializing one field, all other fields are set to 0, so spinlock also.
> > Just choose one field and it's OK.
> > It should be tested with ICC also but I think it's OK.
> 
> Seems that you are right, at least for C99:
> 
>     C99 Standard 6.7.8.21
> 
>          If there are fewer initializers in a brace-enclosed list than
>     there are elements or members of an aggregate, or fewer characters
>     in a string literal used to initialize an array of known size than
>     there are elements in the array, the remainder of the aggregate
>     shall be initialized implicitly the same as objects that have static
>     storage duration.
> 
> 
> I am not sure if there can be problems with other C dialects (e.g. C11),
> I don't have the std here. So to prevent any problem with them (could
> produce a dead-lock during first rte_kni_alloc() that could be difficult
> to troubleshoot), I would still explicitly initialize the mutex, in one
> or the other way.
> 
> Just tell me if you agree and which one you prefer.
> 
> I don't have an ICC license. I am always trying it with GCC and clang.
> 
> Marc

ICC should be fine with this, it handles just initializing a single member of a structure as described by Thomas above.

/Bruce
  

2014-10-22 11:49, Marc Sune:
> On 22/10/14 10:50, Thomas Monjalon wrote:
> > 2014-10-22 10:42, Marc Sune:
> >> The mutex needs to be initialized to RTE_SPINLOCK_INITIALIZER(0) too, or
> >> move the initialization of the mutex to rte_kni_init().
> > RTE_SPINLOCK_INITIALIZER is { 0 }
> > By initializing one field, all other fields are set to 0, so spinlock also.
> > Just choose one field and it's OK.
> > It should be tested with ICC also but I think it's OK.
> 
> Seems that you are right, at least for C99:
> 
>     C99 Standard 6.7.8.21
> 
>          If there are fewer initializers in a brace-enclosed list than
>     there are elements or members of an aggregate, or fewer characters
>     in a string literal used to initialize an array of known size than
>     there are elements in the array, the remainder of the aggregate
>     shall be initialized implicitly the same as objects that have static
>     storage duration.
> 
> 
> I am not sure if there can be problems with other C dialects (e.g. C11), 
> I don't have the std here. So to prevent any problem with them (could 
> produce a dead-lock during first rte_kni_alloc() that could be difficult 
> to troubleshoot), I would still explicitly initialize the mutex, in one 
> or the other way.
> 
> Just tell me if you agree and which one you prefer.

No problem for initializing mutex.

> I don't have an ICC license. I am always trying it with GCC and clang.

That's why it's the Intel job to support ICC in DPDK :)