* [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning @ 2020-05-15 14:28 Kevin Traynor 2020-05-15 19:06 ` Wang, Yipeng1 2020-05-18 11:48 ` David Marchand 0 siblings, 2 replies; 5+ messages in thread From: Kevin Traynor @ 2020-05-15 14:28 UTC (permalink / raw) To: dev, yipeng1.wang, sameh.gobriel, bruce.richardson Cc: honnappa.nagarahalli, ferruh.yigit, david.marchand, Kevin Traynor, stable gcc 10.1.1 reports a warning for the ext_bkt_id variable: ../lib/librte_hash/rte_cuckoo_hash.c: In function ‘__rte_hash_add_key_with_hash’: ../lib/librte_hash/rte_cuckoo_hash.c:1104:29: warning: ‘ext_bkt_id’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1104 | (h->buckets_ext[ext_bkt_id - 1]).sig_current[0] = short_sig; | ~~~~~~~~~~~^~~ The return value of rte_ring_sc_dequeue_elem() is already checked, but also initialize ext_bkt_id to zero (invalid value) and check that it also overwritten. Fixes: fbfe568103b0 ("hash: use 32-bit elements rings to save memory") Cc: stable@dpdk.org Signed-off-by: Kevin Traynor <ktraynor@redhat.com> --- lib/librte_hash/rte_cuckoo_hash.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 38767a8a1..90cb99b0e 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -940,6 +940,6 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, struct rte_hash_bucket *prim_bkt, *sec_bkt, *cur_bkt; struct rte_hash_key *new_k, *keys = h->key_store; + uint32_t ext_bkt_id = 0; uint32_t slot_id; - uint32_t ext_bkt_id; int ret; unsigned n_slots; @@ -1096,5 +1096,6 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, */ if (rte_ring_sc_dequeue_elem(h->free_ext_bkts, &ext_bkt_id, - sizeof(uint32_t)) != 0) { + sizeof(uint32_t)) != 0 || + ext_bkt_id == 0) { ret = -ENOSPC; goto failure; -- 2.21.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning 2020-05-15 14:28 [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning Kevin Traynor @ 2020-05-15 19:06 ` Wang, Yipeng1 2020-05-15 19:17 ` Kevin Traynor 2020-05-18 11:48 ` David Marchand 1 sibling, 1 reply; 5+ messages in thread From: Wang, Yipeng1 @ 2020-05-15 19:06 UTC (permalink / raw) To: Kevin Traynor, dev, Gobriel, Sameh, Richardson, Bruce Cc: honnappa.nagarahalli, Yigit, Ferruh, david.marchand, stable > -----Original Message----- > From: Kevin Traynor <ktraynor@redhat.com> > Sent: Friday, May 15, 2020 7:28 AM > To: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, > Sameh <sameh.gobriel@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com> > Cc: honnappa.nagarahalli@arm.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > david.marchand@redhat.com; Kevin Traynor <ktraynor@redhat.com>; > stable@dpdk.org > Subject: [PATCH] hash: fix gcc 10 maybe-uninitialized warning > > gcc 10.1.1 reports a warning for the ext_bkt_id variable: > > ../lib/librte_hash/rte_cuckoo_hash.c: > In function ‘__rte_hash_add_key_with_hash’: > ../lib/librte_hash/rte_cuckoo_hash.c:1104:29: > warning: ‘ext_bkt_id’ may be used uninitialized in this function [-Wmaybe- > uninitialized] > 1104 | (h->buckets_ext[ext_bkt_id - 1]).sig_current[0] = short_sig; > | ~~~~~~~~~~~^~~ > > The return value of rte_ring_sc_dequeue_elem() is already checked, but > also initialize ext_bkt_id to zero (invalid value) and check that it also > overwritten. > > Fixes: fbfe568103b0 ("hash: use 32-bit elements rings to save memory") > Cc: stable@dpdk.org > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > --- > lib/librte_hash/rte_cuckoo_hash.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_hash/rte_cuckoo_hash.c > b/lib/librte_hash/rte_cuckoo_hash.c > index 38767a8a1..90cb99b0e 100644 > --- a/lib/librte_hash/rte_cuckoo_hash.c > +++ b/lib/librte_hash/rte_cuckoo_hash.c > @@ -940,6 +940,6 @@ __rte_hash_add_key_with_hash(const struct > rte_hash *h, const void *key, > struct rte_hash_bucket *prim_bkt, *sec_bkt, *cur_bkt; > struct rte_hash_key *new_k, *keys = h->key_store; > + uint32_t ext_bkt_id = 0; > uint32_t slot_id; > - uint32_t ext_bkt_id; > int ret; > unsigned n_slots; > @@ -1096,5 +1096,6 @@ __rte_hash_add_key_with_hash(const struct > rte_hash *h, const void *key, > */ > if (rte_ring_sc_dequeue_elem(h->free_ext_bkts, &ext_bkt_id, > - sizeof(uint32_t)) != 0) { > + sizeof(uint32_t)) != 0 || > + ext_bkt_id == 0) { [Wang, Yipeng] If convenient, it would be better to make the two lines aligned with same indent... > ret = -ENOSPC; > goto failure; > -- > 2.21.3 [Wang, Yipeng] Thanks for the fix. I think It is also better code in general. Acked-by: Yipeng Wang <yipeng1.wang@intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning 2020-05-15 19:06 ` Wang, Yipeng1 @ 2020-05-15 19:17 ` Kevin Traynor 2020-05-15 20:04 ` Wang, Yipeng1 0 siblings, 1 reply; 5+ messages in thread From: Kevin Traynor @ 2020-05-15 19:17 UTC (permalink / raw) To: Wang, Yipeng1, dev, Gobriel, Sameh, Richardson, Bruce Cc: honnappa.nagarahalli, Yigit, Ferruh, david.marchand, stable On 15/05/2020 20:06, Wang, Yipeng1 wrote: >> -----Original Message----- >> From: Kevin Traynor <ktraynor@redhat.com> >> Sent: Friday, May 15, 2020 7:28 AM >> To: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, >> Sameh <sameh.gobriel@intel.com>; Richardson, Bruce >> <bruce.richardson@intel.com> >> Cc: honnappa.nagarahalli@arm.com; Yigit, Ferruh <ferruh.yigit@intel.com>; >> david.marchand@redhat.com; Kevin Traynor <ktraynor@redhat.com>; >> stable@dpdk.org >> Subject: [PATCH] hash: fix gcc 10 maybe-uninitialized warning >> >> gcc 10.1.1 reports a warning for the ext_bkt_id variable: >> >> ../lib/librte_hash/rte_cuckoo_hash.c: >> In function ‘__rte_hash_add_key_with_hash’: >> ../lib/librte_hash/rte_cuckoo_hash.c:1104:29: >> warning: ‘ext_bkt_id’ may be used uninitialized in this function [-Wmaybe- >> uninitialized] >> 1104 | (h->buckets_ext[ext_bkt_id - 1]).sig_current[0] = short_sig; >> | ~~~~~~~~~~~^~~ >> >> The return value of rte_ring_sc_dequeue_elem() is already checked, but >> also initialize ext_bkt_id to zero (invalid value) and check that it also >> overwritten. >> >> Fixes: fbfe568103b0 ("hash: use 32-bit elements rings to save memory") >> Cc: stable@dpdk.org >> >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com> >> --- >> lib/librte_hash/rte_cuckoo_hash.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_hash/rte_cuckoo_hash.c >> b/lib/librte_hash/rte_cuckoo_hash.c >> index 38767a8a1..90cb99b0e 100644 >> --- a/lib/librte_hash/rte_cuckoo_hash.c >> +++ b/lib/librte_hash/rte_cuckoo_hash.c >> @@ -940,6 +940,6 @@ __rte_hash_add_key_with_hash(const struct >> rte_hash *h, const void *key, >> struct rte_hash_bucket *prim_bkt, *sec_bkt, *cur_bkt; >> struct rte_hash_key *new_k, *keys = h->key_store; >> + uint32_t ext_bkt_id = 0; >> uint32_t slot_id; >> - uint32_t ext_bkt_id; >> int ret; >> unsigned n_slots; >> @@ -1096,5 +1096,6 @@ __rte_hash_add_key_with_hash(const struct >> rte_hash *h, const void *key, >> */ >> if (rte_ring_sc_dequeue_elem(h->free_ext_bkts, &ext_bkt_id, >> - sizeof(uint32_t)) != 0) { >> + sizeof(uint32_t)) != 0 || >> + ext_bkt_id == 0) { > [Wang, Yipeng] > If convenient, it would be better to make the two lines aligned with same indent... > Hi Yipeng, I had checked the coding style [1] about this and I think it's correct as 'sizeof..' is a wrap from the first condition so gets a second tab to indicate that, whereas 'ext_bkt_id..' is the second condition with no wrap. Fine to change it, if I interpret incorrectly. [1] third bullet, http://doc.dpdk.org/guides/contributing/coding_style.html#general >> ret = -ENOSPC; >> goto failure; >> -- >> 2.21.3 > [Wang, Yipeng] > Thanks for the fix. I think It is also better code in general. > > Acked-by: Yipeng Wang <yipeng1.wang@intel.com> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning 2020-05-15 19:17 ` Kevin Traynor @ 2020-05-15 20:04 ` Wang, Yipeng1 0 siblings, 0 replies; 5+ messages in thread From: Wang, Yipeng1 @ 2020-05-15 20:04 UTC (permalink / raw) To: Kevin Traynor, dev, Gobriel, Sameh, Richardson, Bruce Cc: honnappa.nagarahalli, Yigit, Ferruh, david.marchand, stable > -----Original Message----- > From: Kevin Traynor <ktraynor@redhat.com> > Sent: Friday, May 15, 2020 12:18 PM > To: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@dpdk.org; Gobriel, > Sameh <sameh.gobriel@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com> > Cc: honnappa.nagarahalli@arm.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > david.marchand@redhat.com; stable@dpdk.org > Subject: Re: [PATCH] hash: fix gcc 10 maybe-uninitialized warning > > On 15/05/2020 20:06, Wang, Yipeng1 wrote: > >> -----Original Message----- > >> From: Kevin Traynor <ktraynor@redhat.com> > >> Sent: Friday, May 15, 2020 7:28 AM > >> To: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, > >> Sameh <sameh.gobriel@intel.com>; Richardson, Bruce > >> <bruce.richardson@intel.com> > >> Cc: honnappa.nagarahalli@arm.com; Yigit, Ferruh > >> <ferruh.yigit@intel.com>; david.marchand@redhat.com; Kevin Traynor > >> <ktraynor@redhat.com>; stable@dpdk.org > >> Subject: [PATCH] hash: fix gcc 10 maybe-uninitialized warning > >> > >> gcc 10.1.1 reports a warning for the ext_bkt_id variable: > >> > >> ../lib/librte_hash/rte_cuckoo_hash.c: > >> In function ‘__rte_hash_add_key_with_hash’: > >> ../lib/librte_hash/rte_cuckoo_hash.c:1104:29: > >> warning: ‘ext_bkt_id’ may be used uninitialized in this function > >> [-Wmaybe- uninitialized] > >> 1104 | (h->buckets_ext[ext_bkt_id - 1]).sig_current[0] = short_sig; > >> | ~~~~~~~~~~~^~~ > >> > >> The return value of rte_ring_sc_dequeue_elem() is already checked, > >> but also initialize ext_bkt_id to zero (invalid value) and check that > >> it also overwritten. > >> > >> Fixes: fbfe568103b0 ("hash: use 32-bit elements rings to save > >> memory") > >> Cc: stable@dpdk.org > >> > >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > >> --- > >> lib/librte_hash/rte_cuckoo_hash.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/lib/librte_hash/rte_cuckoo_hash.c > >> b/lib/librte_hash/rte_cuckoo_hash.c > >> index 38767a8a1..90cb99b0e 100644 > >> --- a/lib/librte_hash/rte_cuckoo_hash.c > >> +++ b/lib/librte_hash/rte_cuckoo_hash.c > >> @@ -940,6 +940,6 @@ __rte_hash_add_key_with_hash(const struct > >> rte_hash *h, const void *key, > >> struct rte_hash_bucket *prim_bkt, *sec_bkt, *cur_bkt; > >> struct rte_hash_key *new_k, *keys = h->key_store; > >> + uint32_t ext_bkt_id = 0; > >> uint32_t slot_id; > >> - uint32_t ext_bkt_id; > >> int ret; > >> unsigned n_slots; > >> @@ -1096,5 +1096,6 @@ __rte_hash_add_key_with_hash(const struct > >> rte_hash *h, const void *key, > >> */ > >> if (rte_ring_sc_dequeue_elem(h->free_ext_bkts, &ext_bkt_id, > >> - sizeof(uint32_t)) != 0) { > >> + sizeof(uint32_t)) != 0 || > >> + ext_bkt_id == 0) { > > [Wang, Yipeng] > > If convenient, it would be better to make the two lines aligned with same > indent... > > > > Hi Yipeng, I had checked the coding style [1] about this and I think > it's correct as 'sizeof..' is a wrap from the first condition so gets a > second tab to indicate that, whereas 'ext_bkt_id..' is the second > condition with no wrap. Fine to change it, if I interpret incorrectly. > > [1] third bullet, > http://doc.dpdk.org/guides/contributing/coding_style.html#general [Wang, Yipeng] I see your point now. Thanks for explaining and I think it makes sense to distinguish these two. I guess it just came from my own aesthetic preference. and I don’t have the best coding style : ) > > >> ret = -ENOSPC; > >> goto failure; > >> -- > >> 2.21.3 > > [Wang, Yipeng] > > Thanks for the fix. I think It is also better code in general. > > > > Acked-by: Yipeng Wang <yipeng1.wang@intel.com> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning 2020-05-15 14:28 [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning Kevin Traynor 2020-05-15 19:06 ` Wang, Yipeng1 @ 2020-05-18 11:48 ` David Marchand 1 sibling, 0 replies; 5+ messages in thread From: David Marchand @ 2020-05-18 11:48 UTC (permalink / raw) To: Kevin Traynor Cc: dev, Wang, Yipeng1, Gobriel, Sameh, Bruce Richardson, Honnappa Nagarahalli, Yigit, Ferruh, dpdk stable On Fri, May 15, 2020 at 4:28 PM Kevin Traynor <ktraynor@redhat.com> wrote: > > gcc 10.1.1 reports a warning for the ext_bkt_id variable: > > ../lib/librte_hash/rte_cuckoo_hash.c: > In function ‘__rte_hash_add_key_with_hash’: > ../lib/librte_hash/rte_cuckoo_hash.c:1104:29: > warning: ‘ext_bkt_id’ may be used uninitialized in this function > [-Wmaybe-uninitialized] > 1104 | (h->buckets_ext[ext_bkt_id - 1]).sig_current[0] = short_sig; > | ~~~~~~~~~~~^~~ > > The return value of rte_ring_sc_dequeue_elem() is already checked, > but also initialize ext_bkt_id to zero (invalid value) and check > that it also overwritten. > > Fixes: fbfe568103b0 ("hash: use 32-bit elements rings to save memory") > Cc: stable@dpdk.org > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Yipeng Wang <yipeng1.wang@intel.com> Applied, thanks. -- David Marchand ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-18 11:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-15 14:28 [dpdk-dev] [PATCH] hash: fix gcc 10 maybe-uninitialized warning Kevin Traynor 2020-05-15 19:06 ` Wang, Yipeng1 2020-05-15 19:17 ` Kevin Traynor 2020-05-15 20:04 ` Wang, Yipeng1 2020-05-18 11:48 ` David Marchand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).