Le 12/08/2022 à 08:07, Dan Carpenter a écrit :
On Thu, Aug 11, 2022 at 10:52:13PM +0200, Christophe JAILLET wrote:
Le 11/08/2022 à 12:56, Dan Carpenter a écrit :
This code assumes that platform_get_irq() function returns zero on
failure. In fact, platform_get_irq() never returns zero. It returns
negative error codes or positive non-zero values on success.
Fixes: eca10ae6000d ("watchdog: add driver for Cortina Gemini watchdog")
Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
---
drivers/watchdog/ftwdt010_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/ftwdt010_wdt.c b/drivers/watchdog/ftwdt010_wdt.c
index 21dcc7765688..02112fc264bd 100644
--- a/drivers/watchdog/ftwdt010_wdt.c
+++ b/drivers/watchdog/ftwdt010_wdt.c
@@ -156,7 +156,7 @@ static int ftwdt010_wdt_probe(struct platform_device *pdev)
}
irq = platform_get_irq(pdev, 0);
- if (irq) {
+ if (irq > 0) {
ret = devm_request_irq(dev, irq, ftwdt010_wdt_interrupt, 0,
"watchdog bark", gwdt);
if (ret)
Hi,
can't platform_get_irq() return 0?
All the paths in platform_get_irq() look like 0 is a valid value.
The other patches you just sent are "< 0 ==> error", so ">= 0 ==> valid"
Any reason here for >0?
It can't really be zero. On some of the other patches there was a tests
failure and success. So if we're testing for < 0 then a test for >= 0
felt more complete.
But here it was like, testing for > 0 won't break anything that isn't
already broken. It's easier to review.
Somebody has a Coccinelle test for:
irq = platform_get_irq(pdev, 0);
if (!irq)
return -ENODEV;
But I implemented it in Smatch just for fun. It turns out people had
introduced a couple new bugs recently. Also the it appears that their
Coccinelle test does not warn about for success tests like this one.
And there are still a bunch of test that do:
if (irq <= 0)
return irq ?: -ENODEV;
But testing for zero is dead code so I didn't bother cleaning it up.
regards,
dan carpenter
Got it, thanks.
CJ
