Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, 6 Jan 2013, Anton Vorontsov wrote: > On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: > > On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: > > > The patch is whitespace-damaged (for some reason there are two spaces in > > > the beginning of each non-change line). I repeated changes manually, but > > > you might want to fix your mail/patch setup anyway. :) > > > > > > > It may be something on your end, the patch applies fine for me. > > I just looked into Julia's email headers, and it says: > > Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed > > flowed? for the patch? :) That's not right. > > Documentation/email-clients.txt says: > > Don't send patches with "format=flowed". This can cause unexpected > and unwanted line breaks. That is what I fixed. I had used that mailer before for sending patches, and even fixed the problem before, so I have no idea why it changed... But it is fixed now. thanks, julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, 6 Jan 2013, Anton Vorontsov wrote: On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: The patch is whitespace-damaged (for some reason there are two spaces in the beginning of each non-change line). I repeated changes manually, but you might want to fix your mail/patch setup anyway. :) It may be something on your end, the patch applies fine for me. I just looked into Julia's email headers, and it says: Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed flowed? for the patch? :) That's not right. Documentation/email-clients.txt says: Don't send patches with format=flowed. This can cause unexpected and unwanted line breaks. That is what I fixed. I had used that mailer before for sending patches, and even fixed the problem before, so I have no idea why it changed... But it is fixed now. thanks, julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 10:53:21PM -0800, Anton Vorontsov wrote: > On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: > > On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: > > > The patch is whitespace-damaged (for some reason there are two spaces in > > > the beginning of each non-change line). I repeated changes manually, but > > > you might want to fix your mail/patch setup anyway. :) > > > > > > > It may be something on your end, the patch applies fine for me. > > I just looked into Julia's email headers, and it says: > > Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed > > flowed? for the patch? :) That's not right. > > Documentation/email-clients.txt says: > > Don't send patches with "format=flowed". This can cause unexpected > and unwanted line breaks. > > :-P > > As for why it applied fine for you, your mailer probably automatically > decodes the content, but I apply mbox file directly. (If I '-C' the > email in mutt, then it also applies for me, but I normally don't do that.) Ah. You're right, of course. I'm also using mutt but I have in my macros like you say. macro index,pager ap "rm -f tmp/patchtmp/patch~/progs/kernel/apply_patch.sh ~/var/mail/tmp/patch" When I pipe to scripts it uses the undecoded copy. I wish I knew how to use the decoded version there as well... regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: > On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: > > The patch is whitespace-damaged (for some reason there are two spaces in > > the beginning of each non-change line). I repeated changes manually, but > > you might want to fix your mail/patch setup anyway. :) > > > > It may be something on your end, the patch applies fine for me. I just looked into Julia's email headers, and it says: Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed flowed? for the patch? :) That's not right. Documentation/email-clients.txt says: Don't send patches with "format=flowed". This can cause unexpected and unwanted line breaks. :-P As for why it applied fine for you, your mailer probably automatically decodes the content, but I apply mbox file directly. (If I '-C' the email in mutt, then it also applies for me, but I normally don't do that.) Anton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Mon, 7 Jan 2013, Dan Carpenter wrote: > On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: > > The patch is whitespace-damaged (for some reason there are two spaces in > > the beginning of each non-change line). I repeated changes manually, but > > you might want to fix your mail/patch setup anyway. :) > > > > It may be something on your end, the patch applies fine for me. There was a problem on my end and I fixed it. thanks, julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: > The patch is whitespace-damaged (for some reason there are two spaces in > the beginning of each non-change line). I repeated changes manually, but > you might want to fix your mail/patch setup anyway. :) > It may be something on your end, the patch applies fine for me. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 09:43:10AM +0100, Julia Lawall wrote:
> From: Julia Lawall
>
> devm_kzalloc should not be followed by kfree, as this results in a double
> free. The problem was found using the following semantic match
> (http://coccinelle.lip6.fr/):
>
> //
> @@
> expression x,e;
> @@
> x = devm_kzalloc(...)
> ... when != x = e
> ?-kfree(x,...);
> //
>
> Furthermore, in the remove function, the calls to free_irq are moved up to
> prevent a possible reference in the interrupt handler to resources freed by
> power_supply_unregister.
>
> Signed-off-by: Julia Lawall
>
> ---
> V2, moving up the calls to free_irq, instead of removing them and using
> devm_request_threaded_irq.
The patch is whitespace-damaged (for some reason there are two spaces in
the beginning of each non-change line). I repeated changes manually, but
you might want to fix your mail/patch setup anyway. :)
Thanks.
> drivers/power/88pm860x_battery.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/power/88pm860x_battery.c
> b/drivers/power/88pm860x_battery.c
> index 8bc80b0..d338c1c 100644
> --- a/drivers/power/88pm860x_battery.c
> +++ b/drivers/power/88pm860x_battery.c
> @@ -915,15 +915,13 @@ static int pm860x_battery_probe(struct platform_device
> *pdev)
> info->irq_cc = platform_get_irq(pdev, 0);
> if (info->irq_cc <= 0) {
> dev_err(>dev, "No IRQ resource!\n");
> - ret = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> info->irq_batt = platform_get_irq(pdev, 1);
> if (info->irq_batt <= 0) {
> dev_err(>dev, "No IRQ resource!\n");
> - ret = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> info->chip = chip;
> @@ -957,7 +955,7 @@ static int pm860x_battery_probe(struct platform_device
> *pdev)
>
> ret = power_supply_register(>dev, >battery);
> if (ret)
> - goto out;
> + return ret;
> info->battery.dev->parent = >dev;
>
> ret = request_threaded_irq(info->irq_cc, NULL,
> @@ -984,8 +982,6 @@ out_coulomb:
> free_irq(info->irq_cc, info);
> out_reg:
> power_supply_unregister(>battery);
> -out:
> - kfree(info);
> return ret;
> }
>
> @@ -993,10 +989,9 @@ static int pm860x_battery_remove(struct platform_device
> *pdev)
> {
> struct pm860x_battery_info *info = platform_get_drvdata(pdev);
>
> - power_supply_unregister(>battery);
> free_irq(info->irq_batt, info);
> free_irq(info->irq_cc, info);
> - kfree(info);
> + power_supply_unregister(>battery);
> platform_set_drvdata(pdev, NULL);
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 09:43:10AM +0100, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
devm_kzalloc should not be followed by kfree, as this results in a double
free. The problem was found using the following semantic match
(http://coccinelle.lip6.fr/):
// smpl
@@
expression x,e;
@@
x = devm_kzalloc(...)
... when != x = e
?-kfree(x,...);
// /smpl
Furthermore, in the remove function, the calls to free_irq are moved up to
prevent a possible reference in the interrupt handler to resources freed by
power_supply_unregister.
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
V2, moving up the calls to free_irq, instead of removing them and using
devm_request_threaded_irq.
The patch is whitespace-damaged (for some reason there are two spaces in
the beginning of each non-change line). I repeated changes manually, but
you might want to fix your mail/patch setup anyway. :)
Thanks.
drivers/power/88pm860x_battery.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/power/88pm860x_battery.c
b/drivers/power/88pm860x_battery.c
index 8bc80b0..d338c1c 100644
--- a/drivers/power/88pm860x_battery.c
+++ b/drivers/power/88pm860x_battery.c
@@ -915,15 +915,13 @@ static int pm860x_battery_probe(struct platform_device
*pdev)
info-irq_cc = platform_get_irq(pdev, 0);
if (info-irq_cc = 0) {
dev_err(pdev-dev, No IRQ resource!\n);
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
info-irq_batt = platform_get_irq(pdev, 1);
if (info-irq_batt = 0) {
dev_err(pdev-dev, No IRQ resource!\n);
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
info-chip = chip;
@@ -957,7 +955,7 @@ static int pm860x_battery_probe(struct platform_device
*pdev)
ret = power_supply_register(pdev-dev, info-battery);
if (ret)
- goto out;
+ return ret;
info-battery.dev-parent = pdev-dev;
ret = request_threaded_irq(info-irq_cc, NULL,
@@ -984,8 +982,6 @@ out_coulomb:
free_irq(info-irq_cc, info);
out_reg:
power_supply_unregister(info-battery);
-out:
- kfree(info);
return ret;
}
@@ -993,10 +989,9 @@ static int pm860x_battery_remove(struct platform_device
*pdev)
{
struct pm860x_battery_info *info = platform_get_drvdata(pdev);
- power_supply_unregister(info-battery);
free_irq(info-irq_batt, info);
free_irq(info-irq_cc, info);
- kfree(info);
+ power_supply_unregister(info-battery);
platform_set_drvdata(pdev, NULL);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: The patch is whitespace-damaged (for some reason there are two spaces in the beginning of each non-change line). I repeated changes manually, but you might want to fix your mail/patch setup anyway. :) It may be something on your end, the patch applies fine for me. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Mon, 7 Jan 2013, Dan Carpenter wrote: On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: The patch is whitespace-damaged (for some reason there are two spaces in the beginning of each non-change line). I repeated changes manually, but you might want to fix your mail/patch setup anyway. :) It may be something on your end, the patch applies fine for me. There was a problem on my end and I fixed it. thanks, julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: The patch is whitespace-damaged (for some reason there are two spaces in the beginning of each non-change line). I repeated changes manually, but you might want to fix your mail/patch setup anyway. :) It may be something on your end, the patch applies fine for me. I just looked into Julia's email headers, and it says: Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed flowed? for the patch? :) That's not right. Documentation/email-clients.txt says: Don't send patches with format=flowed. This can cause unexpected and unwanted line breaks. :-P As for why it applied fine for you, your mailer probably automatically decodes the content, but I apply mbox file directly. (If I 'ESC-C' the email in mutt, then it also applies for me, but I normally don't do that.) Anton -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/power/88pm860x_battery.c: eliminate possible references to released resources
On Sun, Jan 06, 2013 at 10:53:21PM -0800, Anton Vorontsov wrote: On Mon, Jan 07, 2013 at 09:42:23AM +0300, Dan Carpenter wrote: On Sun, Jan 06, 2013 at 01:16:50PM -0800, Anton Vorontsov wrote: The patch is whitespace-damaged (for some reason there are two spaces in the beginning of each non-change line). I repeated changes manually, but you might want to fix your mail/patch setup anyway. :) It may be something on your end, the patch applies fine for me. I just looked into Julia's email headers, and it says: Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed flowed? for the patch? :) That's not right. Documentation/email-clients.txt says: Don't send patches with format=flowed. This can cause unexpected and unwanted line breaks. :-P As for why it applied fine for you, your mailer probably automatically decodes the content, but I apply mbox file directly. (If I 'ESC-C' the email in mutt, then it also applies for me, but I normally don't do that.) Ah. You're right, of course. I'm also using mutt but I have decode-copy in my macros like you say. macro index,pager ap shell-escaperm -f tmp/patchenterdecode-copytmp/patchentershell-escape~/progs/kernel/apply_patch.sh ~/var/mail/tmp/patchenter When I pipe to scripts it uses the undecoded copy. I wish I knew how to use the decoded version there as well... regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
