Message ID | 1413961851-13230-1-git-send-email-marc.sune@bisdn.de (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 0D39E7E95; Wed, 22 Oct 2014 09:03:00 +0200 (CEST) Received: from mx.bisdn.de (mx.bisdn.de [185.27.182.31]) by dpdk.org (Postfix) with ESMTP id A9CAC7E80 for <dev@dpdk.org>; Wed, 22 Oct 2014 09:02:52 +0200 (CEST) Received: from localhost.localdomain (unknown [172.16.251.36]) by mx.bisdn.de (Postfix) with ESMTP id E203BA2DA9; Wed, 22 Oct 2014 09:11:10 +0200 (CEST) From: Marc Sune <marc.sune@bisdn.de> To: dev@dpdk.org Date: Wed, 22 Oct 2014 09:10:51 +0200 Message-Id: <1413961851-13230-1-git-send-email-marc.sune@bisdn.de> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1ED644BD7E0A5F4091CF203DAFB8E4CC01D8288F@SHSMSX101.ccr.corp.intel.com> References: <1ED644BD7E0A5F4091CF203DAFB8E4CC01D8288F@SHSMSX101.ccr.corp.intel.com> Subject: [dpdk-dev] [PATCH] KNI: fix compilation warning 'missing-field-initializers' X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Marc Sune
Oct. 22, 2014, 7:10 a.m. UTC
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(-)
Comments
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 :)
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,