* [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation failure
@ 2010-10-15 12:54 Julia Lawall
2010-10-15 14:24 ` [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation walter harms
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2010-10-15 12:54 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: kernel-janitors, devel, linux-kernel
From: Julia Lawall <julia@diku.dk>
Each of these functions defines a variable err and a variable retval. err
is always 0 and is returned in the error case. At one point near the end
of the function, retval is initialized to an error code. For each
function, the patch drops err, stores -ENOMEM in retval in two cases where
memory allocation fails, and changes the return value in the error case
from err to retval.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression ret;
expression x,e1,e2,e3;
@@
ret = 0
... when != ret = e1
*x = \(kmalloc\|kcalloc\|kzalloc\)(...)
... when != ret = e2
if (x = NULL) { ... when != ret = e3
return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/staging/cx25821/cx25821-audio-upstream.c | 11 +++++++----
drivers/staging/cx25821/cx25821-video-upstream-ch2.c | 11 +++++++----
drivers/staging/cx25821/cx25821-video-upstream.c | 11 +++++++----
3 files changed, 21 insertions(+), 12 deletions(-)
diff -u -p a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
--- a/drivers/staging/cx25821/cx25821-audio-upstream.c
+++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
@@ -723,7 +723,6 @@ int cx25821_audio_upstream_init(struct c
{
struct sram_channel *sram_ch;
int retval = 0;
- int err = 0;
int str_length = 0;
if (dev->_audio_is_running) {
@@ -756,8 +755,10 @@ int cx25821_audio_upstream_init(struct c
str_length = strlen(dev->input_audiofilename);
dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_audiofilename)
+ if (!dev->_audiofilename) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_audiofilename, dev->input_audiofilename,
str_length + 1);
@@ -770,8 +771,10 @@ int cx25821_audio_upstream_init(struct c
str_length = strlen(_defaultAudioName);
dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_audiofilename)
+ if (!dev->_audiofilename) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_audiofilename, _defaultAudioName, str_length + 1);
}
@@ -802,5 +805,5 @@ int cx25821_audio_upstream_init(struct c
error:
cx25821_dev_unregister(dev);
- return err;
+ return retval;
}
diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
--- a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
+++ b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
@@ -747,7 +747,6 @@ int cx25821_vidupstream_init_ch2(struct
struct sram_channel *sram_ch;
u32 tmp;
int retval = 0;
- int err = 0;
int data_frame_size = 0;
int risc_buffer_size = 0;
int str_length = 0;
@@ -792,8 +791,10 @@ int cx25821_vidupstream_init_ch2(struct
str_length = strlen(dev->input_filename_ch2);
dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_filename_ch2)
+ if (!dev->_filename_ch2) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_filename_ch2, dev->input_filename_ch2,
str_length + 1);
@@ -801,8 +802,10 @@ int cx25821_vidupstream_init_ch2(struct
str_length = strlen(dev->_defaultname_ch2);
dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_filename_ch2)
+ if (!dev->_filename_ch2) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_filename_ch2, dev->_defaultname_ch2,
str_length + 1);
@@ -851,5 +854,5 @@ int cx25821_vidupstream_init_ch2(struct
error:
cx25821_dev_unregister(dev);
- return err;
+ return retval;
}
diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream.c b/drivers/staging/cx25821/cx25821-video-upstream.c
--- a/drivers/staging/cx25821/cx25821-video-upstream.c
+++ b/drivers/staging/cx25821/cx25821-video-upstream.c
@@ -800,7 +800,6 @@ int cx25821_vidupstream_init_ch1(struct
struct sram_channel *sram_ch;
u32 tmp;
int retval = 0;
- int err = 0;
int data_frame_size = 0;
int risc_buffer_size = 0;
int str_length = 0;
@@ -844,16 +843,20 @@ int cx25821_vidupstream_init_ch1(struct
str_length = strlen(dev->input_filename);
dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_filename)
+ if (!dev->_filename) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_filename, dev->input_filename, str_length + 1);
} else {
str_length = strlen(dev->_defaultname);
dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
- if (!dev->_filename)
+ if (!dev->_filename) {
+ retval = -ENOMEM;
goto error;
+ }
memcpy(dev->_filename, dev->_defaultname, str_length + 1);
}
@@ -908,5 +911,5 @@ int cx25821_vidupstream_init_ch1(struct
error:
cx25821_dev_unregister(dev);
- return err;
+ return retval;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation
2010-10-15 12:54 [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation failure Julia Lawall
@ 2010-10-15 14:24 ` walter harms
2010-10-15 19:57 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: walter harms @ 2010-10-15 14:24 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, devel, linux-kernel
This looks like a case for kasprintf().
re
wh
Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
>
> Each of these functions defines a variable err and a variable retval. err
> is always 0 and is returned in the error case. At one point near the end
> of the function, retval is initialized to an error code. For each
> function, the patch drops err, stores -ENOMEM in retval in two cases where
> memory allocation fails, and changes the return value in the error case
> from err to retval.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression ret;
> expression x,e1,e2,e3;
> @@
>
> ret = 0
> ... when != ret = e1
> *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
> ... when != ret = e2
> if (x = NULL) { ... when != ret = e3
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> drivers/staging/cx25821/cx25821-audio-upstream.c | 11 +++++++----
> drivers/staging/cx25821/cx25821-video-upstream-ch2.c | 11 +++++++----
> drivers/staging/cx25821/cx25821-video-upstream.c | 11 +++++++----
> 3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff -u -p a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
> --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
> +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
> @@ -723,7 +723,6 @@ int cx25821_audio_upstream_init(struct c
> {
> struct sram_channel *sram_ch;
> int retval = 0;
> - int err = 0;
> int str_length = 0;
>
> if (dev->_audio_is_running) {
> @@ -756,8 +755,10 @@ int cx25821_audio_upstream_init(struct c
> str_length = strlen(dev->input_audiofilename);
> dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_audiofilename)
> + if (!dev->_audiofilename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_audiofilename, dev->input_audiofilename,
> str_length + 1);
> @@ -770,8 +771,10 @@ int cx25821_audio_upstream_init(struct c
> str_length = strlen(_defaultAudioName);
> dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_audiofilename)
> + if (!dev->_audiofilename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_audiofilename, _defaultAudioName, str_length + 1);
> }
> @@ -802,5 +805,5 @@ int cx25821_audio_upstream_init(struct c
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> --- a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> +++ b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> @@ -747,7 +747,6 @@ int cx25821_vidupstream_init_ch2(struct
> struct sram_channel *sram_ch;
> u32 tmp;
> int retval = 0;
> - int err = 0;
> int data_frame_size = 0;
> int risc_buffer_size = 0;
> int str_length = 0;
> @@ -792,8 +791,10 @@ int cx25821_vidupstream_init_ch2(struct
> str_length = strlen(dev->input_filename_ch2);
> dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename_ch2)
> + if (!dev->_filename_ch2) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename_ch2, dev->input_filename_ch2,
> str_length + 1);
> @@ -801,8 +802,10 @@ int cx25821_vidupstream_init_ch2(struct
> str_length = strlen(dev->_defaultname_ch2);
> dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename_ch2)
> + if (!dev->_filename_ch2) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename_ch2, dev->_defaultname_ch2,
> str_length + 1);
> @@ -851,5 +854,5 @@ int cx25821_vidupstream_init_ch2(struct
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream.c b/drivers/staging/cx25821/cx25821-video-upstream.c
> --- a/drivers/staging/cx25821/cx25821-video-upstream.c
> +++ b/drivers/staging/cx25821/cx25821-video-upstream.c
> @@ -800,7 +800,6 @@ int cx25821_vidupstream_init_ch1(struct
> struct sram_channel *sram_ch;
> u32 tmp;
> int retval = 0;
> - int err = 0;
> int data_frame_size = 0;
> int risc_buffer_size = 0;
> int str_length = 0;
> @@ -844,16 +843,20 @@ int cx25821_vidupstream_init_ch1(struct
> str_length = strlen(dev->input_filename);
> dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename)
> + if (!dev->_filename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename, dev->input_filename, str_length + 1);
> } else {
> str_length = strlen(dev->_defaultname);
> dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename)
> + if (!dev->_filename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename, dev->_defaultname, str_length + 1);
> }
> @@ -908,5 +911,5 @@ int cx25821_vidupstream_init_ch1(struct
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> --
> 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] 4+ messages in thread
* Re: [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation
2010-10-15 14:24 ` [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation walter harms
@ 2010-10-15 19:57 ` Julia Lawall
2010-10-17 18:44 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2010-10-15 19:57 UTC (permalink / raw)
To: walter harms; +Cc: kernel-janitors, devel, linux-kernel
On Fri, 15 Oct 2010, walter harms wrote:
> This looks like a case for kasprintf().
OK, I will take a look at it.
julia
> re
> wh
>
> Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Each of these functions defines a variable err and a variable retval. err
> > is always 0 and is returned in the error case. At one point near the end
> > of the function, retval is initialized to an error code. For each
> > function, the patch drops err, stores -ENOMEM in retval in two cases where
> > memory allocation fails, and changes the return value in the error case
> > from err to retval.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression ret;
> > expression x,e1,e2,e3;
> > @@
> >
> > ret = 0
> > ... when != ret = e1
> > *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
> > ... when != ret = e2
> > if (x = NULL) { ... when != ret = e3
> > return ret;
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> >
> > ---
> > drivers/staging/cx25821/cx25821-audio-upstream.c | 11 +++++++----
> > drivers/staging/cx25821/cx25821-video-upstream-ch2.c | 11 +++++++----
> > drivers/staging/cx25821/cx25821-video-upstream.c | 11 +++++++----
> > 3 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff -u -p a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
> > --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
> > +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
> > @@ -723,7 +723,6 @@ int cx25821_audio_upstream_init(struct c
> > {
> > struct sram_channel *sram_ch;
> > int retval = 0;
> > - int err = 0;
> > int str_length = 0;
> >
> > if (dev->_audio_is_running) {
> > @@ -756,8 +755,10 @@ int cx25821_audio_upstream_init(struct c
> > str_length = strlen(dev->input_audiofilename);
> > dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_audiofilename)
> > + if (!dev->_audiofilename) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_audiofilename, dev->input_audiofilename,
> > str_length + 1);
> > @@ -770,8 +771,10 @@ int cx25821_audio_upstream_init(struct c
> > str_length = strlen(_defaultAudioName);
> > dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_audiofilename)
> > + if (!dev->_audiofilename) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_audiofilename, _defaultAudioName, str_length + 1);
> > }
> > @@ -802,5 +805,5 @@ int cx25821_audio_upstream_init(struct c
> > error:
> > cx25821_dev_unregister(dev);
> >
> > - return err;
> > + return retval;
> > }
> > diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> > --- a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> > +++ b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> > @@ -747,7 +747,6 @@ int cx25821_vidupstream_init_ch2(struct
> > struct sram_channel *sram_ch;
> > u32 tmp;
> > int retval = 0;
> > - int err = 0;
> > int data_frame_size = 0;
> > int risc_buffer_size = 0;
> > int str_length = 0;
> > @@ -792,8 +791,10 @@ int cx25821_vidupstream_init_ch2(struct
> > str_length = strlen(dev->input_filename_ch2);
> > dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_filename_ch2)
> > + if (!dev->_filename_ch2) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_filename_ch2, dev->input_filename_ch2,
> > str_length + 1);
> > @@ -801,8 +802,10 @@ int cx25821_vidupstream_init_ch2(struct
> > str_length = strlen(dev->_defaultname_ch2);
> > dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_filename_ch2)
> > + if (!dev->_filename_ch2) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_filename_ch2, dev->_defaultname_ch2,
> > str_length + 1);
> > @@ -851,5 +854,5 @@ int cx25821_vidupstream_init_ch2(struct
> > error:
> > cx25821_dev_unregister(dev);
> >
> > - return err;
> > + return retval;
> > }
> > diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream.c b/drivers/staging/cx25821/cx25821-video-upstream.c
> > --- a/drivers/staging/cx25821/cx25821-video-upstream.c
> > +++ b/drivers/staging/cx25821/cx25821-video-upstream.c
> > @@ -800,7 +800,6 @@ int cx25821_vidupstream_init_ch1(struct
> > struct sram_channel *sram_ch;
> > u32 tmp;
> > int retval = 0;
> > - int err = 0;
> > int data_frame_size = 0;
> > int risc_buffer_size = 0;
> > int str_length = 0;
> > @@ -844,16 +843,20 @@ int cx25821_vidupstream_init_ch1(struct
> > str_length = strlen(dev->input_filename);
> > dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_filename)
> > + if (!dev->_filename) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_filename, dev->input_filename, str_length + 1);
> > } else {
> > str_length = strlen(dev->_defaultname);
> > dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
> >
> > - if (!dev->_filename)
> > + if (!dev->_filename) {
> > + retval = -ENOMEM;
> > goto error;
> > + }
> >
> > memcpy(dev->_filename, dev->_defaultname, str_length + 1);
> > }
> > @@ -908,5 +911,5 @@ int cx25821_vidupstream_init_ch1(struct
> > error:
> > cx25821_dev_unregister(dev);
> >
> > - return err;
> > + return retval;
> > }
> > --
> > 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] 4+ messages in thread
* Re: [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation
2010-10-15 19:57 ` Julia Lawall
@ 2010-10-17 18:44 ` Julia Lawall
0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2010-10-17 18:44 UTC (permalink / raw)
To: walter harms; +Cc: kernel-janitors, devel, linux-kernel
On Fri, 15 Oct 2010, Julia Lawall wrote:
> On Fri, 15 Oct 2010, walter harms wrote:
>
> > This looks like a case for kasprintf().
>
> OK, I will take a look at it.
Instead, I have found some other occurrences of kmalloc + memcpy that
could be kasprintf, so I have done them all in another patch series, with
the subject "Use kasprintf".
julia
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-10-17 18:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-15 12:54 [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation failure Julia Lawall
2010-10-15 14:24 ` [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation walter harms
2010-10-15 19:57 ` Julia Lawall
2010-10-17 18:44 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox