Skip to contentSkip to conversation

da9052-battery: don't free IRQ that wasn't requested

PATCH Applied Dan Carpenter 3 participants 4 messages Sep 5, 2012 – Sep 20, 2012 Kernel janitors

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 da9052-battery: don't free IRQ that wasn't requested

We should decrement "i" before doing the free_irq(). If we call this because request_threaded_irq() failed then we don't want to free the thing which failed. Or in the case where we get here because power_supply_register() failed then the original codes does a read past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/power/da9052-battery.c +1 -1
index 20b86ed..d9d034d 100644
@@ -623,7 +623,7 @@ static s32 __devinit da9052_bat_probe(struct platform_device *pdev)
 	return 0;
 
 err:
-	for (; i >= 0; i--) {
+	while (--i >= 0) {
 		irq = platform_get_irq_byname(pdev, da9052_bat_irqs[i]);
 		free_irq(bat->da9052->irq_base + irq, bat);
 	}
#1 lore raw

Am 05.09.2012 14:34, schrieb Dan Carpenter:

Quoted 13 lines

We should decrement "i" before doing the free_irq(). If we call this because request_threaded_irq() failed then we don't want to free the thing which failed. Or in the case where we get here because power_supply_register() failed then the original codes does a read past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/power/da9052-battery.c +0 -0
index 20b86ed..d9d034d 100644

drivers/power/da9052-battery.c static s32 __devinit da9052_bat_probe(struct platform_device *pdev) · line 629

Quoted 8 lines
drivers/power/da9052-battery.c +1 -1
@@ -623,7 +623,7 @@ static s32 __devinit da9052_bat_probe(struct platform_device *pdev)
 	return 0;
 
 err:
-	for (; i >= 0; i--) {
+	while (--i >= 0) {
 		irq = platform_get_irq_byname(pdev, da9052_bat_irqs[i]);
 		free_irq(bat->da9052->irq_base + irq, bat);
 	}

hi da,
(my usual nitpicking ...)
since a lot of people do make mistakes on count-down-loops, is there any chance to make this a common count-up-for()-loop ?
like:

   for (j=0; j <= i ;j++ ) {
	irq = platform_get_irq_byname(pdev, da9052_bat_irqs[j]);
	free_irq(bat->da9052->irq_base + irq, bat);
	}

re,
wh

author
#2 lore raw
Quoted 18 lines · walter harms

Am 05.09.2012 14:34, schrieb Dan Carpenter:

Quoted 15 lines

We should decrement "i" before doing the free_irq(). If we call this because request_threaded_irq() failed then we don't want to free the thing which failed. Or in the case where we get here because power_supply_register() failed then the original codes does a read past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/power/da9052-battery.c +0 -0
index 20b86ed..d9d034d 100644
@@ -623,7 +623,7 @@ static s32 __devinit da9052_bat_probe(struct platform_device *pdev)
 	return 0;
Quoted 9 lines

drivers/power/da9052-battery.c static s32 __devinit da9052_bat_probe(struct platform_device *pdev) · line 629

Quoted 6 lines
drivers/power/da9052-battery.c +1 -1
 err:
-	for (; i >= 0; i--) {
+	while (--i >= 0) {
 		irq = platform_get_irq_byname(pdev, da9052_bat_irqs[i]);
 		free_irq(bat->da9052->irq_base + irq, bat);
 	}

hi da,
(my usual nitpicking ...)

Ha ha. Your nit picks are welcome even if I don't always agree.

Quoted 3 lines

since a lot of people do make mistakes on count-down-loops, is there any chance to make this a common count-up-for()-loop ?
like:

I like the count down loops... It feels very natural to unwind that way.

Quoted 1 line
   for (j=0; j <= i ;j++ ) {
               ^^^^^^

The count up loops are prone to the exact same off by one bugs. ;) You've got one in your sample code. Plus I'd have to declare another variable and send a v2 patch and I am very lazy... So in this case I think should just take my original patch.

regards,
dan carpenter

Applied
#3 lore raw

On Thu, Sep 06, 2012 at 08:07:58AM -0700, Dan Carpenter wrote:
[...]

Quoted 6 lines
Quoted 1 line
   for (j=0; j <= i ;j++ ) {
               ^^^^^^

The count up loops are prone to the exact same off by one bugs. ;) You've got one in your sample code. Plus I'd have to declare another variable and send a v2 patch and I am very lazy... So in this case I think should just take my original patch.

Applied, thank you!