Re: [dpdk-dev] [PATCH 06/10] net/dpaa2: fix possible use of uninitialized vars

Re: [dpdk-dev] [PATCH 06/10] net/dpaa2: fix possible use of uninitialized vars

[Andrzej Ostruszka]

On 9/5/19 4:10 PM, Hemant Agrawal wrote:
> Hi Andrzei,
> 
>> -----Original Message-----
> 
>> diff --git a/drivers/net/dpaa2/mc/dpni.c b/drivers/net/dpaa2/mc/dpni.c
>> index 362cd476f..b74a1a317 100644
>> --- a/drivers/net/dpaa2/mc/dpni.c
>> +++ b/drivers/net/dpaa2/mc/dpni.c
>> @@ -1803,10 +1803,13 @@ int dpni_set_congestion_notification(struct
>> fsl_mc_io *mc_io,
>>  	cmd_params->qtype = qtype;
>>  	cmd_params->tc = tc_id;
>>  	cmd_params->congestion_point = cfg->cg_point;
>> -	cmd_params->cgid = (uint8_t)cfg->cgid;
>> -	cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
>> +	if (cfg->cg_point == DPNI_CP_CONGESTION_GROUP)
>> +		cmd_params->cgid = (uint8_t)cfg->cgid;
>> +	if (cfg->dest_cfg.dest_type != DPNI_DEST_NONE) {
>> +		cmd_params->dest_id = cpu_to_le32(cfg-
>>> dest_cfg.dest_id);
>> +		cmd_params->dest_priority = cfg->dest_cfg.priority;
>> +	}
> 
> [Hemant] what was the exact issues you are facing in this piece of code? 
> Please share your compiler and other env details. 
> 
>>  	cmd_params->notification_mode = cpu_to_le16(cfg-
>>> notification_mode);
>> -	cmd_params->dest_priority = cfg->dest_cfg.priority;

That was warning from compiler that neither of these:
- cfg->cgid
- cfg->dest_cfg.dest_id
- cfg->dest_cfg.dest_priority
is initialized in dpaa2_dev_tx_queue_setup() where this function is
called and they are used here.  This might be non issue - I've changed
the function here however since I though it would be better - but I
don't know this code, I'm just guessing that:
- cgid is meaningful only when cg_point is DPNI_CP_CONGESTION_GROUP
- dest_id/dest_priority are meaningful when dest_type is not DPNI_DEST_NONE.

That was developed on 19.05 and rebased to 19.11 and I was using gcc
(7.4) however today when I commented out those 'if' I did not got the
warning which I wanted to send here.  I'll try to figure out what has
changed that this is no longer reported (I think the compiler should
still report this).

Best regards
Andrzej

BTW.  In my original submission I've made a mistake with DPDK dev
e-mail, so I'm replying now with it corrected.

[dpdk-dev] [PATCH 06/10] net/dpaa2: fix possible use of uninitialized vars

[Andrzej Ostruszka]

This patch fixes 'maybe-uninitialized' warnings reported by compiler
when using LTO.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 drivers/net/dpaa2/base/dpaa2_hw_dpni.c | 1 +
 drivers/net/dpaa2/mc/dpkg.c            | 2 +-
 drivers/net/dpaa2/mc/dpni.c            | 9 ++++++---
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa2/base/dpaa2_hw_dpni.c b/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
index 56e2e56a3..2f6534a31 100644
--- a/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
+++ b/drivers/net/dpaa2/base/dpaa2_hw_dpni.c
@@ -51,6 +51,7 @@ rte_pmd_dpaa2_set_custom_hash(uint16_t port_id,
 	kg_cfg.extracts[0].type = DPKG_EXTRACT_FROM_DATA;
 	kg_cfg.extracts[0].extract.from_data.offset = offset;
 	kg_cfg.extracts[0].extract.from_data.size = size;
+	kg_cfg.extracts[0].num_of_byte_masks = 0;
 	kg_cfg.num_extracts = 1;
 
 	ret = dpkg_prepare_key_cfg(&kg_cfg, p_params);
diff --git a/drivers/net/dpaa2/mc/dpkg.c b/drivers/net/dpaa2/mc/dpkg.c
index 80f94f40e..7aa63ea12 100644
--- a/drivers/net/dpaa2/mc/dpkg.c
+++ b/drivers/net/dpaa2/mc/dpkg.c
@@ -63,7 +63,7 @@ dpkg_prepare_key_cfg(const struct dpkg_profile_cfg *cfg, uint8_t *key_cfg_buf)
 		dpkg_set_field(extr->extract_type, EXTRACT_TYPE,
 			       cfg->extracts[i].type);
 
-		for (j = 0; j < DPKG_NUM_OF_MASKS; j++) {
+		for (j = 0; j < extr->num_of_byte_masks; j++) {
 			extr->masks[j].mask = cfg->extracts[i].masks[j].mask;
 			extr->masks[j].offset =
 				cfg->extracts[i].masks[j].offset;
diff --git a/drivers/net/dpaa2/mc/dpni.c b/drivers/net/dpaa2/mc/dpni.c
index 362cd476f..b74a1a317 100644
--- a/drivers/net/dpaa2/mc/dpni.c
+++ b/drivers/net/dpaa2/mc/dpni.c
@@ -1803,10 +1803,13 @@ int dpni_set_congestion_notification(struct fsl_mc_io *mc_io,
 	cmd_params->qtype = qtype;
 	cmd_params->tc = tc_id;
 	cmd_params->congestion_point = cfg->cg_point;
-	cmd_params->cgid = (uint8_t)cfg->cgid;
-	cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
+	if (cfg->cg_point == DPNI_CP_CONGESTION_GROUP)
+		cmd_params->cgid = (uint8_t)cfg->cgid;
+	if (cfg->dest_cfg.dest_type != DPNI_DEST_NONE) {
+		cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
+		cmd_params->dest_priority = cfg->dest_cfg.priority;
+	}
 	cmd_params->notification_mode = cpu_to_le16(cfg->notification_mode);
-	cmd_params->dest_priority = cfg->dest_cfg.priority;
 	cmd_params->message_iova = cpu_to_le64(cfg->message_iova);
 	cmd_params->message_ctx = cpu_to_le64(cfg->message_ctx);
 	cmd_params->threshold_entry = cpu_to_le32(cfg->threshold_entry);
-- 
2.17.1