char: xilinx_hwicap: missing error code if ioremap() fails

PATCH In review Dan Carpenter 3 participants 5 messages Jun 11, 2014 – Jun 17, 2014

Subsystems (3)

Kernel janitors Char and misc drivers Arm/Zynq architecture

lore

Share & tools

lore

Dan Carpenter

author

2014-06-11 06:04 UTC #0 lore raw

PATCH 1/1 char: xilinx_hwicap: missing error code if ioremap() fails

Return -ENOMEM instead of success if ioremap() fails.

Signed-off-by Dan Carpenter <[email protected]>

---
Static checker stuff, I can't compile this.

drivers/char/xilinx_hwicap/xilinx_hwicap.c +1 -0

index f6345f9..9b1a5ac 100644
@@ -661,6 +661,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->base_address = ioremap(drvdata->mem_start, drvdata->mem_size);
 	if (!drvdata->base_address) {
 		dev_err(dev, "ioremap() failed\n");
+		retval = -ENOMEM;
 		goto failed2;
 	}

Jingoo Han

Reviewed-by on v1 1/1

2014-06-11 06:40 UTC #1 lore raw

Quoted 4 lines · Dan Carpenter

Return -ENOMEM instead of success if ioremap() fails.
Signed-off-by Dan Carpenter <[email protected]>

Reviewed-by Jingoo Han <[email protected]>

Best regards,
Jingoo Han

Quoted 15 lines

---
Static checker stuff, I can't compile this.

drivers/char/xilinx_hwicap/xilinx_hwicap.c +1 -0

index f6345f9..9b1a5ac 100644
@@ -661,6 +661,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->base_address = ioremap(drvdata->mem_start, drvdata->mem_size);
 	if (!drvdata->base_address) {
 		dev_err(dev, "ioremap() failed\n");
+		retval = -ENOMEM;
 		goto failed2;
 	}

Michal Simek

2014-06-17 00:53 UTC #2 lore raw

Quoted 14 lines · Jingoo Han

Quoted 4 lines · Dan Carpenter
Return -ENOMEM instead of success if ioremap() fails.
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jingoo Han <[email protected]>
Best regards,
Jingoo Han
Quoted 3 lines
---
Static checker stuff, I can't compile this.

Quoted 9 lines

drivers/char/xilinx_hwicap/xilinx_hwicap.c static int hwicap_setup(struct device *dev, int id, · line 664
Quoted 9 lines
drivers/char/xilinx_hwicap/xilinx_hwicap.c +1 -0
index f6345f9..9b1a5ac 100644
@@ -661,6 +661,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->base_address = ioremap(drvdata->mem_start, drvdata->mem_size);
 	if (!drvdata->base_address) {
 		dev_err(dev, "ioremap() failed\n");
+		retval = -ENOMEM;

Isn't it just better to do?

if (IS_ERR(drvdata->base_address)) {
	...
	retval = PTR_ERR(drvdata->base_address);
}

Thanks,
Michal

Dan Carpenter

author

2014-06-17 06:03 UTC #3 lore raw

Quoted 27 lines · Michal Simek

Quoted 14 lines · Jingoo Han
Quoted 4 lines · Dan Carpenter
Return -ENOMEM instead of success if ioremap() fails.
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jingoo Han <[email protected]>
Best regards,
Jingoo Han
Quoted 3 lines
---
Static checker stuff, I can't compile this.
Quoted 9 lines
drivers/char/xilinx_hwicap/xilinx_hwicap.c static int hwicap_setup(struct device *dev, int id, · line 664
Quoted 9 lines
drivers/char/xilinx_hwicap/xilinx_hwicap.c +1 -0
index f6345f9..9b1a5ac 100644
@@ -661,6 +661,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->base_address = ioremap(drvdata->mem_start, drvdata->mem_size);
 	if (!drvdata->base_address) {
 		dev_err(dev, "ioremap() failed\n");
+		retval = -ENOMEM;
Isn't it just better to do?

Quoted 4 lines

if (IS_ERR(drvdata->base_address)) {
	...
	retval = PTR_ERR(drvdata->base_address);
}

ioremap() returns NULL on error, not an ERR_PTR.

regards,
dan carpenter

Michal Simek

2014-06-17 22:11 UTC #4 lore raw

Quoted 28 lines · Dan Carpenter

Quoted 27 lines · Michal Simek
Quoted 14 lines · Jingoo Han
Quoted 4 lines · Dan Carpenter
Return -ENOMEM instead of success if ioremap() fails.
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jingoo Han <[email protected]>
Best regards,
Jingoo Han
Quoted 3 lines
---
Static checker stuff, I can't compile this.
Quoted 9 lines
drivers/char/xilinx_hwicap/xilinx_hwicap.c static int hwicap_setup(struct device *dev, int id, · line 664
Quoted 9 lines
drivers/char/xilinx_hwicap/xilinx_hwicap.c +1 -0
index f6345f9..9b1a5ac 100644
@@ -661,6 +661,7 @@ static int hwicap_setup(struct device *dev, int id,
 	drvdata->base_address = ioremap(drvdata->mem_start, drvdata->mem_size);
 	if (!drvdata->base_address) {
 		dev_err(dev, "ioremap() failed\n");
+		retval = -ENOMEM;
Isn't it just better to do?

Quoted 6 lines

Quoted 4 lines
if (IS_ERR(drvdata->base_address)) {
	...
	retval = PTR_ERR(drvdata->base_address);
}
ioremap() returns NULL on error, not an ERR_PTR.

Ah ok. Fair enough.

Thanks,
Michal