DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] hash: fix possible uninitialized variable
@ 2018-12-22 12:10 Haiyang Tan
  2018-12-23  7:43 ` Stephen Hemminger
  0 siblings, 1 reply; 3+ messages in thread
From: Haiyang Tan @ 2018-12-22 12:10 UTC (permalink / raw)
  To: Cristian Dumitrescu; +Cc: dev, Haiyang Tan

The uninitialized field 'extra_flag' of hash_cuckoo_params may enable
certain feature silently. Typically, if bit0 of 'extra_flag' set, the
hardware transactional memory support will be enabled unexpectedly.

Signed-off-by: Haiyang Tan <haiyangtan@tencent.com>
---
 lib/librte_table/rte_table_hash_cuckoo.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/librte_table/rte_table_hash_cuckoo.c b/lib/librte_table/rte_table_hash_cuckoo.c
index f02430333..25fc3a10b 100644
--- a/lib/librte_table/rte_table_hash_cuckoo.c
+++ b/lib/librte_table/rte_table_hash_cuckoo.c
@@ -82,6 +82,7 @@ rte_table_hash_cuckoo_create(void *params,
 			uint32_t entry_size)
 {
 	struct rte_table_hash_cuckoo_params *p = params;
+	struct rte_hash_parameters hash_cuckoo_params = { 0 };
 	struct rte_hash *h_table;
 	struct rte_table_hash *t;
 	uint32_t total_size;
@@ -103,14 +104,12 @@ rte_table_hash_cuckoo_create(void *params,
 	}
 
 	/* Create cuckoo hash table */
-	struct rte_hash_parameters hash_cuckoo_params = {
-		.entries = p->n_keys,
-		.key_len = p->key_size,
-		.hash_func = p->f_hash,
-		.hash_func_init_val = p->seed,
-		.socket_id = socket_id,
-		.name = p->name
-	};
+	hash_cuckoo_params.entries = p->n_keys;
+	hash_cuckoo_params.key_len = p->key_size;
+	hash_cuckoo_params.hash_func = p->f_hash;
+	hash_cuckoo_params.hash_func_init_val = p->seed;
+	hash_cuckoo_params.socket_id = socket_id;
+	hash_cuckoo_params.name = p->name;
 
 	h_table = rte_hash_find_existing(p->name);
 	if (h_table == NULL) {
-- 
2.14.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] hash: fix possible uninitialized variable
  2018-12-22 12:10 [dpdk-dev] [PATCH] hash: fix possible uninitialized variable Haiyang Tan
@ 2018-12-23  7:43 ` Stephen Hemminger
  2018-12-24 10:30   ` Dumitrescu, Cristian
  0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2018-12-23  7:43 UTC (permalink / raw)
  To: Haiyang Tan; +Cc: Cristian Dumitrescu, dev

On Sat, 22 Dec 2018 04:10:59 -0800
Haiyang Tan <haiyangtan@tencent.com> wrote:

> The uninitialized field 'extra_flag' of hash_cuckoo_params may enable
> certain feature silently. Typically, if bit0 of 'extra_flag' set, the
> hardware transactional memory support will be enabled unexpectedly.
> 
> Signed-off-by: Haiyang Tan <haiyangtan@tencent.com>

This is not necessary. Structure initializations will fill in the other
elements with zero.

https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Designated-Inits.html
  Omitted field members are implicitly initialized the same as objects that have static storage duration.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] hash: fix possible uninitialized variable
  2018-12-23  7:43 ` Stephen Hemminger
@ 2018-12-24 10:30   ` Dumitrescu, Cristian
  0 siblings, 0 replies; 3+ messages in thread
From: Dumitrescu, Cristian @ 2018-12-24 10:30 UTC (permalink / raw)
  To: Stephen Hemminger, Haiyang Tan; +Cc: dev



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Sunday, December 23, 2018 7:44 AM
> To: Haiyang Tan <haiyangtan@tencent.com>
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] hash: fix possible uninitialized variable
> 
> On Sat, 22 Dec 2018 04:10:59 -0800
> Haiyang Tan <haiyangtan@tencent.com> wrote:
> 
> > The uninitialized field 'extra_flag' of hash_cuckoo_params may enable
> > certain feature silently. Typically, if bit0 of 'extra_flag' set, the
> > hardware transactional memory support will be enabled unexpectedly.
> >
> > Signed-off-by: Haiyang Tan <haiyangtan@tencent.com>
> 
> This is not necessary. Structure initializations will fill in the other
> elements with zero.
> 
> https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Designated-Inits.html
>   Omitted field members are implicitly initialized the same as objects that
> have static storage duration.

Agree with Stephen, this is part of the C language.

Haiyang, are you experiencing a real issue in your app or is your proposal triggered purely by code review?

Haiyang, it seems that the extra_flags was recently added in librte_hash, but left behind in librte_table. For better readability, I suggest you send a quick patch that explicitly initializes the extra_flag with 0, what do you think?

	struct rte_hash_parameters hash_cuckoo_params = {
		...
		.extra_flag = 0,
	};

Regards,
Cristian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-24 10:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-22 12:10 [dpdk-dev] [PATCH] hash: fix possible uninitialized variable Haiyang Tan
2018-12-23  7:43 ` Stephen Hemminger
2018-12-24 10:30   ` Dumitrescu, Cristian

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).