Skip to contentSkip to conversation

remoteproc: silence an uninitialized variable warning

PATCH In review Dan Carpenter 2 participants 5 messages Apr 4, 2019 – Apr 5, 2019

Subsystems (3)
Arm Qualcomm mailing list Kernel janitors Arm Qualcomm support
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 remoteproc: silence an uninitialized variable warning

Smatch complains that "ret" might be uninitialized. I can see why it generates the warning, but I don't know if it's actually possible. Anyway initializing "ret" here is harmless.

Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/soc/qcom/mdt_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/soc/qcom/mdt_loader.c +1 -1
index 1c488024c698..fc58d660692f 100644
@@ -188,7 +188,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
 
 	if (reloc_base)
 		*reloc_base = mem_reloc;
-
+	ret = 0;
 out:
 	kfree(fw_name);
Signature · 1 line
2.17.1
#1 lore raw
Quoted 14 lines · Dan Carpenter

Smatch complains that "ret" might be uninitialized. I can see why it generates the warning, but I don't know if it's actually possible. Anyway initializing "ret" here is harmless.

Signed-off-by Dan Carpenter <[email protected]>
---
  drivers/soc/qcom/mdt_loader.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/soc/qcom/mdt_loader.c +0 -0
index 1c488024c698..fc58d660692f 100644

drivers/soc/qcom/mdt_loader.c static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, · line 191

Quoted 5 lines
drivers/soc/qcom/mdt_loader.c +1 -1
@@ -188,7 +188,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
 
  	if (reloc_base)
  		*reloc_base = mem_reloc;
-
+	ret = 0;

You are overriding the value here, better keep it at the start. Thanks,
Mukesh

Quoted 3 lines
drivers/soc/qcom/mdt_loader.c +0 -0
  out:
  	kfree(fw_name);
author
#2 lore raw
Quoted 11 lines · Mukesh Ojha
Quoted 9 lines · Dan Carpenter

Smatch complains that "ret" might be uninitialized. I can see why it generates the warning, but I don't know if it's actually possible. Anyway initializing "ret" here is harmless.

Signed-off-by Dan Carpenter <[email protected]>
---
  drivers/soc/qcom/mdt_loader.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines

drivers/soc/qcom/mdt_loader.c static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, · line 190

Quoted 9 lines
drivers/soc/qcom/mdt_loader.c +1 -1
index 1c488024c698..fc58d660692f 100644
@@ -188,7 +188,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
  	if (reloc_base)
  		*reloc_base = mem_reloc;
-
+	ret = 0;

You are overriding the value here, better keep it at the start.

I like how I wrote it. It makes it clear that this is the success path.

regards,
dan carpenter

#3 lore raw
Quoted 22 lines · Dan Carpenter
Quoted 10 lines · Mukesh Ojha
Quoted 9 lines · Dan Carpenter

Smatch complains that "ret" might be uninitialized. I can see why it generates the warning, but I don't know if it's actually possible. Anyway initializing "ret" here is harmless.

Signed-off-by Dan Carpenter <[email protected]>
---
   drivers/soc/qcom/mdt_loader.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 11 lines

drivers/soc/qcom/mdt_loader.c static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, · line 190

Quoted 9 lines
drivers/soc/qcom/mdt_loader.c +1 -1
index 1c488024c698..fc58d660692f 100644
@@ -188,7 +188,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
   	if (reloc_base)
   		*reloc_base = mem_reloc;
-
+	ret = 0;

You are overriding the value here, better keep it at the start.

Quoted 1 line

I like how I wrote it. It makes it clear that this is the success path.

But think about a case when this request_firmware_into_buf fails or the snippet above that can set the ret with error value

right.. is not that possible?..and ret = 0 stops propagating the error properly.

Thanks
Mukesh

Quoted 4 lines

regards,
dan carpenter

author
#4 lore raw
Quoted 24 lines · Mukesh Ojha
Quoted 22 lines · Dan Carpenter
Quoted 21 lines · Mukesh Ojha
Quoted 14 lines · Dan Carpenter

Smatch complains that "ret" might be uninitialized. I can see why it generates the warning, but I don't know if it's actually possible. Anyway initializing "ret" here is harmless.

Signed-off-by Dan Carpenter <[email protected]>
---
   drivers/soc/qcom/mdt_loader.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
drivers/soc/qcom/mdt_loader.c +0 -0
index 1c488024c698..fc58d660692f 100644

drivers/soc/qcom/mdt_loader.c static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, · line 190

Quoted 4 lines
drivers/soc/qcom/mdt_loader.c +1 -1
@@ -188,7 +188,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
   	if (reloc_base)
   		*reloc_base = mem_reloc;
-
+	ret = 0;

You are overriding the value here, better keep it at the start.

Quoted 9 lines
Quoted 1 line

I like how I wrote it. It makes it clear that this is the success path.

But think about a case when this request_firmware_into_buf fails or the snippet above that can set the ret with error value

right.. is not that possible?..and ret = 0 stops propagating the error properly.

Gar. I read "goto out;" instead of "break;". It probably should be goto out though... Anyway, I will resend.

Thanks!

regards,
dan carpenter