Skip to contentSkip to conversation

regulator: ab8500 - fix the logic to remove already registered regulators in error path

PATCH Applied Axel Lin 5 participants 6 messages Aug 14, 2010 – Aug 18, 2010 Voltage and current regulator framework

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 regulator: ab8500 - fix the logic to remove already registered regulators in error path

In current implementation, ab8500_regulator_info[0].regulator is not unregistered if the error happen at i > 0.

This patch fixes the resource leak and also improves the readability.

---
I think this change is better ( in readability ) than simply change "while (i > 0)" to "while (i >= 0)".

 drivers/regulator/ab8500.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/regulator/ab8500.c +1 -3
index dc3f1a4..775688e 100644
@@ -368,11 +368,9 @@ static __devinit int ab8500_regulator_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 					info->desc.name);
 			/* when we fail, un-register all earlier regulators */
-			i--;
-			while (i > 0) {
+			while (--i >= 0) {
 				info = &ab8500_regulator_info[i];
 				regulator_unregister(info->regulator);
-				i--;
 			}
 			return err;
 		}
Signature · 1 line
1.7.0.4
#1 lore raw

Thanx!

BTW, could you also include the AB3100 as well into this patch? I derived it from the AB3100 implementation.

Linus, any comments?

Regards,
Sundar

Quoted 39 lines

-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Saturday, August 14, 2010 7:14 PM
To: linux-kernel
Cc: Liam Girdwood; Mark Brown; Sundar R IYER
Subject: [PATCH] regulator: ab8500 - fix the logic to remove already registered regulators in error path

In current implementation, ab8500_regulator_info[0].regulator is not unregistered if the error happen at i > 0.

This patch fixes the resource leak and also improves the readability.

---
I think this change is better ( in readability ) than simply change "while (i > 0)" to "while (i >= 0)".

drivers/regulator/ab8500.c | 4 +---

1 files changed, 1 insertions(+), 3 deletions(-)
drivers/regulator/ab8500.c +1 -3
index dc3f1a4..775688e 100644
@@ -368,11 +368,9 @@ static __devinit int ab8500_regulator_probe(struct
 platform_device *pdev)
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 					info->desc.name);
 			/* when we fail, un-register all earlier regulators */
-			i--;
-			while (i > 0) {
+			while (--i >= 0) {
 				info = &ab8500_regulator_info[i];
 				regulator_unregister(info->regulator);
-				i--;
 			}
 			return err;
		}
Quoted 4 lines

--
1.7.0.4

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

Acked-by
#2 lore raw

2010/8/14 Sundar R IYER <[email protected]>:

Quoted 2 lines

BTW, could you also include the AB3100 as well into this patch? I derived it from the AB3100 implementation.

Aha the same thing for AB8500, yeah I confess it is probably my coding mistake replicated so

on this one too then!

Thanks.
Linus Walleij

#3 lore raw
In reply to Axel Lin

Oops, I didn’t see the below patch.

[PATCH] regulator: ab3100 - fix the logic to remove already registered regulators in error path

Sorry for the inconvenience!

Cheers!
Sundar

Quoted 53 lines

-----Original Message-----
From: Sundar R IYER
Sent: Saturday, August 14, 2010 8:05 PM
To: 'Axel Lin'; linux-kernel
Cc: Liam Girdwood; Mark Brown; Linus WALLEIJ
Subject: RE: [PATCH] regulator: ab8500 - fix the logic to remove already registered regulators in error path

Thanx!

BTW, could you also include the AB3100 as well into this patch? I derived it from the AB3100 implementation.

Linus, any comments?

Regards,
Sundar

Quoted 35 lines

-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Saturday, August 14, 2010 7:14 PM
To: linux-kernel
Cc: Liam Girdwood; Mark Brown; Sundar R IYER
Subject: [PATCH] regulator: ab8500 - fix the logic to remove already registered regulators in error path

In current implementation, ab8500_regulator_info[0].regulator is not unregistered if the error happen at i > 0.

This patch fixes the resource leak and also improves the readability.

---
I think this change is better ( in readability ) than simply change "while (i > 0)" to "while (i >= 0)".

drivers/regulator/ab8500.c | 4 +---

1 files changed, 1 insertions(+), 3 deletions(-)
drivers/regulator/ab8500.c +1 -2
index dc3f1a4..775688e 100644
@@ -368,11 +368,9 @@ static __devinit int ab8500_regulator_probe(struct
 platform_device *pdev)
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 					info->desc.name);
 			/* when we fail, un-register all earlier regulators */
-			i--;
-			while (i > 0) {
+			while (--i >= 0) {
 				info = &ab8500_regulator_info[i];
 				regulator_unregister(info->regulator);
Quoted 8 lines
Quoted 8 lines
drivers/regulator/ab8500.c +0 -1
-				i--;
 			}
 			return err;
		}

--
1.7.0.4

���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

#5 lore raw
In reply to Axel Lin
Quoted 19 lines · Axel Lin

In current implementation, ab8500_regulator_info[0].regulator is not unregistered if the error happen at i > 0.

This patch fixes the resource leak and also improves the readability.

---
I think this change is better ( in readability ) than simply change "while (i > 0)" to "while (i >= 0)".

 drivers/regulator/ab8500.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/regulator/ab8500.c +0 -0
index dc3f1a4..775688e 100644
@@ -368,11 +368,9 @@ static __devinit int ab8500_regulator_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 					info->desc.name);

drivers/regulator/ab8500.c static __devinit int ab8500_regulator_probe(struct platform_device *pdev) · line 376

Quoted 10 lines
drivers/regulator/ab8500.c +1 -3
 			/* when we fail, un-register all earlier regulators */
-			i--;
-			while (i > 0) {
+			while (--i >= 0) {
 				info = &ab8500_regulator_info[i];
 				regulator_unregister(info->regulator);
-				i--;
 			}
 			return err;
 		}

Applied.

Thanks

Liam

Signature · 3 lines
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk