dma: imx-dma: fix signedness bug

PATCH In review Vasiliy Kulikov 3 participants 5 messages Oct 17, 2010 – Oct 20, 2010

Subsystems (3)

Arm i.MX and Layerscape Kernel janitors dmaengine

lore

Share & tools

lore

Vasiliy Kulikov

author

2010-10-17 14:51 UTC #0 lore raw

PATCH 1/1 dma: imx-dma: fix signedness bug

mxdmac->channel was unsigned, so check (imxdmac->channel < 0) for failed imx_dma_request_by_prio() made no sence. Explicitly check signed values.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
 drivers/dma/imx-dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/dma/imx-dma.c +1 -1

index 346be62..75d870f 100644
@@ -335,7 +335,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
 		imxdmac->imxdma_channel = imx_dma_request_by_prio("dmaengine",
 				DMA_PRIO_MEDIUM);
-		if (imxdmac->channel < 0)
+		if ((int)imxdmac->channel < 0)
 			goto err_init;
 
 		imx_dma_setup_handlers(imxdmac->imxdma_channel,

Signature · 1 line

1.7.0.4

Sascha Hauer

maintainer Acked-by on v1 1/1

2010-10-18 08:02 UTC #1 lore raw

Quoted 5 lines · Vasiliy Kulikov

mxdmac->channel was unsigned, so check (imxdmac->channel < 0) for failed imx_dma_request_by_prio() made no sence. Explicitly check signed values.
Signed-off-by Vasiliy Kulikov <[email protected]>

Acked-by Sascha Hauer <[email protected]>

Quoted 21 lines

---
 drivers/dma/imx-dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/dma/imx-dma.c +1 -1

index 346be62..75d870f 100644
@@ -335,7 +335,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
 		imxdmac->imxdma_channel = imx_dma_request_by_prio("dmaengine",
 				DMA_PRIO_MEDIUM);
-		if (imxdmac->channel < 0)
+		if ((int)imxdmac->channel < 0)
 			goto err_init;
 
 		imx_dma_setup_handlers(imxdmac->imxdma_channel,

Signature · 1 line

1.7.0.4

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Dan Williams

2010-10-19 22:26 UTC #2 lore raw

In reply to Vasiliy Kulikov

Quoted 14 lines · Vasiliy Kulikov

mxdmac->channel was unsigned, so check (imxdmac->channel < 0) for failed imx_dma_request_by_prio() made no sence. Explicitly check signed values.
Signed-off-by Vasiliy Kulikov <[email protected]>
---
drivers/dma/imx-dma.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/dma/imx-dma.c +0 -0
index 346be62..75d870f 100644

drivers/dma/imx-dma.c static int __init imxdma_probe(struct platform_device *pdev) · line 339

Quoted 6 lines

drivers/dma/imx-dma.c +1 -1

@@ -335,7 +335,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
                imxdmac->imxdma_channel = imx_dma_request_by_prio("dmaengine",
                                DMA_PRIO_MEDIUM);
-               if (imxdmac->channel < 0)
+               if ((int)imxdmac->channel < 0)
                        goto err_init;

Should this be:

               if ((int)imxdmac->channel < 0) {
                       ret = -ENODEV;
                       goto err_init;
               }

...because I get the following in my compilation tests. drivers/dma/imx-dma.c: In function 'imxdma_probe':
drivers/dma/imx-dma.c:324: warning: 'ret' may be used uninitialized in this function

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sascha Hauer

maintainer

2010-10-20 06:35 UTC #3 lore raw

Quoted 24 lines · Dan Williams

Quoted 14 lines · Vasiliy Kulikov
mxdmac->channel was unsigned, so check (imxdmac->channel < 0) for failed imx_dma_request_by_prio() made no sence. Explicitly check signed values.
Signed-off-by Vasiliy Kulikov <[email protected]>
---
drivers/dma/imx-dma.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/dma/imx-dma.c +0 -0
index 346be62..75d870f 100644
drivers/dma/imx-dma.c static int __init imxdma_probe(struct platform_device *pdev) · line 339
Quoted 6 lines
drivers/dma/imx-dma.c +1 -1
@@ -335,7 +335,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
                imxdmac->imxdma_channel = imx_dma_request_by_prio("dmaengine",
                                DMA_PRIO_MEDIUM);
-               if (imxdmac->channel < 0)
+               if ((int)imxdmac->channel < 0)
                        goto err_init;
Should this be:

Quoted 6 lines

               if ((int)imxdmac->channel < 0) {
                       ret = -ENODEV;
                       goto err_init;
               }

...because I get the following in my compilation tests.

Yes, ret is clearly used uninitialized. I wonder why my gcc-4.3.2 compiler does not warn me about this :(

Sascha

Signature · 8 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sascha Hauer

maintainer

2010-10-20 06:37 UTC #4 lore raw

In reply to Dan Williams

mxdmac->channel was unsigned, so check (imxdmac->channel < 0) for failed imx_dma_request_by_prio() made no sence. Explicitly check signed values.
Also, fix uninitialzed use of ret.

Signed-off-by Vasiliy Kulikov <[email protected]>

Signed-off-by Sascha Hauer <[email protected]>

---

Here's an updated patch fixing both issues.

 drivers/dma/imx-dma.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

drivers/dma/imx-dma.c +3 -1

index 346be62..f629e49 100644
@@ -335,8 +335,10 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
 		imxdmac->imxdma_channel = imx_dma_request_by_prio("dmaengine",
 				DMA_PRIO_MEDIUM);
-		if (imxdmac->channel < 0)
+		if ((int)imxdmac->channel < 0) {
+			ret = -ENODEV;
 			goto err_init;
+		}
 
 		imx_dma_setup_handlers(imxdmac->imxdma_channel,
 		       imxdma_irq_handler, imxdma_err_handler, imxdmac);

Signature · 7 lines

1.7.2.3

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |