* [PATCH 4/6] drivers/acpi: Use kasprintf
@ 2010-03-10 21:16 Julia Lawall
2010-03-18 23:25 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2010-03-10 21:16 UTC (permalink / raw)
To: Zhang Rui, Len Brown, linux-acpi, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
kasprintf combines kmalloc and sprintf, and takes care of the size
calculation itself.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression a,flag;
expression list args;
statement S;
@@
a - \(kmalloc\|kzalloc\)(...,flag)
+ kasprintf(flag,args)
<... when != a
if (a = NULL || ...) S
...>
- sprintf(a,args);
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/acpi/video.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
result = acpi_video_init_brightness(device);
if (result)
return;
- name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+ name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
if (!name)
return;
- sprintf(name, "acpi_video%d", count++);
device->backlight = backlight_device_register(name,
NULL, device, &acpi_backlight_ops);
kfree(name);
@@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
if (device->cap._DCS && device->cap._DSS) {
static int count;
char *name;
- name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+ name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
if (!name)
return;
- sprintf(name, "acpi_video%d", count++);
device->output_dev = video_output_register(name,
NULL, device, &acpi_output_properties);
kfree(name);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 4/6] drivers/acpi: Use kasprintf
2010-03-10 21:16 [PATCH 4/6] drivers/acpi: Use kasprintf Julia Lawall
@ 2010-03-18 23:25 ` Andrew Morton
2010-03-18 23:48 ` Julia Lawall
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2010-03-18 23:25 UTC (permalink / raw)
To: Julia Lawall
Cc: Zhang Rui, Len Brown, linux-acpi, linux-kernel, kernel-janitors
On Wed, 10 Mar 2010 22:16:51 +0100 (CET)
Julia Lawall <julia@diku.dk> wrote:
> From: Julia Lawall <julia@diku.dk>
>
> kasprintf combines kmalloc and sprintf, and takes care of the size
> calculation itself.
>
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression a,flag;
> expression list args;
> statement S;
> @@
>
> a > - \(kmalloc\|kzalloc\)(...,flag)
> + kasprintf(flag,args)
> <... when != a
> if (a = NULL || ...) S
> ...>
> - sprintf(a,args);
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> drivers/acpi/video.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
> result = acpi_video_init_brightness(device);
> if (result)
> return;
> - name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> + name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> if (!name)
> return;
>
> - sprintf(name, "acpi_video%d", count++);
> device->backlight = backlight_device_register(name,
> NULL, device, &acpi_backlight_ops);
> kfree(name);
> @@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
> if (device->cap._DCS && device->cap._DSS) {
> static int count;
> char *name;
> - name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> + name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> if (!name)
> return;
> - sprintf(name, "acpi_video%d", count++);
> device->output_dev = video_output_register(name,
> NULL, device, &acpi_output_properties);
> kfree(name);
It's not an equivalent change - if the memory allocation fails, this
patch will now cause `count' to be incremented anyway, presumably
leaving a gap in the enumeration.
I don't know if that matters. If so, this:
--- a/drivers/acpi/video.c~drivers-acpi-use-kasprintf-fix
+++ a/drivers/acpi/video.c
@@ -1006,9 +1006,10 @@ static void acpi_video_device_find_cap(s
result = acpi_video_init_brightness(device);
if (result)
return;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
+ name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
if (!name)
return;
+ count++;
memset(&props, 0, sizeof(struct backlight_properties));
props.max_brightness = device->brightness->count - 3;
@@ -1058,9 +1059,10 @@ static void acpi_video_device_find_cap(s
if (device->cap._DCS && device->cap._DSS) {
static int count;
char *name;
- name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
+ name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
if (!name)
return;
+ count++;
device->output_dev = video_output_register(name,
NULL, device, &acpi_output_properties);
kfree(name);
_
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 4/6] drivers/acpi: Use kasprintf
2010-03-18 23:25 ` Andrew Morton
@ 2010-03-18 23:48 ` Julia Lawall
0 siblings, 0 replies; 3+ messages in thread
From: Julia Lawall @ 2010-03-18 23:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Zhang Rui, Len Brown, linux-acpi, linux-kernel, kernel-janitors
On Thu, 18 Mar 2010, Andrew Morton wrote:
> On Wed, 10 Mar 2010 22:16:51 +0100 (CET)
> Julia Lawall <julia@diku.dk> wrote:
>
> > From: Julia Lawall <julia@diku.dk>
> >
> > kasprintf combines kmalloc and sprintf, and takes care of the size
> > calculation itself.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression a,flag;
> > expression list args;
> > statement S;
> > @@
> >
> > a > > - \(kmalloc\|kzalloc\)(...,flag)
> > + kasprintf(flag,args)
> > <... when != a
> > if (a = NULL || ...) S
> > ...>
> > - sprintf(a,args);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> >
> > ---
> > drivers/acpi/video.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c
> > --- a/drivers/acpi/video.c
> > +++ b/drivers/acpi/video.c
> > @@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
> > result = acpi_video_init_brightness(device);
> > if (result)
> > return;
> > - name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> > + name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> > if (!name)
> > return;
> >
> > - sprintf(name, "acpi_video%d", count++);
> > device->backlight = backlight_device_register(name,
> > NULL, device, &acpi_backlight_ops);
> > kfree(name);
> > @@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
> > if (device->cap._DCS && device->cap._DSS) {
> > static int count;
> > char *name;
> > - name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> > + name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> > if (!name)
> > return;
> > - sprintf(name, "acpi_video%d", count++);
> > device->output_dev = video_output_register(name,
> > NULL, device, &acpi_output_properties);
> > kfree(name);
>
> It's not an equivalent change - if the memory allocation fails, this
> patch will now cause `count' to be incremented anyway, presumably
> leaving a gap in the enumeration.
Indeed. What is below looks better.
julia
> I don't know if that matters. If so, this:
>
> --- a/drivers/acpi/video.c~drivers-acpi-use-kasprintf-fix
> +++ a/drivers/acpi/video.c
> @@ -1006,9 +1006,10 @@ static void acpi_video_device_find_cap(s
> result = acpi_video_init_brightness(device);
> if (result)
> return;
> - name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> + name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> if (!name)
> return;
> + count++;
>
> memset(&props, 0, sizeof(struct backlight_properties));
> props.max_brightness = device->brightness->count - 3;
> @@ -1058,9 +1059,10 @@ static void acpi_video_device_find_cap(s
> if (device->cap._DCS && device->cap._DSS) {
> static int count;
> char *name;
> - name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
> + name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> if (!name)
> return;
> + count++;
> device->output_dev = video_output_register(name,
> NULL, device, &acpi_output_properties);
> kfree(name);
> _
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-03-18 23:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-10 21:16 [PATCH 4/6] drivers/acpi: Use kasprintf Julia Lawall
2010-03-18 23:25 ` Andrew Morton
2010-03-18 23:48 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox