2010-07-28 16:41:57

by Kulikov Vasiliy

Subject: [PATCH 09/10] oss: msnd: check request_region() return value

request_region() may fail, if so return -ENOMEM.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
sound/oss/msnd_pinnacle.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c
index bfaac5f..cd70b9a 100644
--- a/sound/oss/msnd_pinnacle.c
+++ b/sound/oss/msnd_pinnacle.c
@@ -1400,9 +1400,13 @@ static int __init attach_multisound(void)
printk(KERN_ERR LOGNAME ": Couldn't grab IRQ %d\n", dev.irq);
return err;
}
- request_region(dev.io, dev.numio, dev.name);
+ if (request_region(dev.io, dev.numio, dev.name) == NULL) {
+ free_irq(dev.irq, &dev);
+ return -ENOMEM;
+ }

- if ((err = dsp_full_reset()) < 0) {
+ err = dsp_full_reset();
+ if (err < 0) {
release_region(dev.io, dev.numio);
free_irq(dev.irq, &dev);
return err;
--
1.7.0.4

2010-07-28 20:00:31

by Dan Carpenter

Subject: Re: [PATCH 09/10] oss: msnd: check request_region() return value

On Wed, Jul 28, 2010 at 08:41:36PM +0400, Kulikov Vasiliy wrote:
> request_region() may fail, if so return -ENOMEM.
>
> Signed-off-by: Kulikov Vasiliy <[email protected]>
> ---
> sound/oss/msnd_pinnacle.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c
> index bfaac5f..cd70b9a 100644
> --- a/sound/oss/msnd_pinnacle.c
> +++ b/sound/oss/msnd_pinnacle.c
> @@ -1400,9 +1400,13 @@ static int __init attach_multisound(void)
> printk(KERN_ERR LOGNAME ": Couldn't grab IRQ %d\n", dev.irq);
> return err;
> }
> - request_region(dev.io, dev.numio, dev.name);

This should be -EBUSY as well. The same for "[PATCH 06/10] ALSA: msnd:
check request_region() return value"

Another way to write that would be:

- request_region(dev.io, dev.numio, dev.name);
+ err = request_region(dev.io, dev.numio, dev.name);
+ if (err) {
+ free_irq(dev.irq, &dev);
+ return err;
+ }

regards,
dan carpenter

> + if (request_region(dev.io, dev.numio, dev.name) == NULL) {
> + free_irq(dev.irq, &dev);
> + return -ENOMEM;
> + }

2010-07-28 21:21:11

by Dan Carpenter

Subject: Re: [PATCH 09/10] oss: msnd: check request_region() return value

On Wed, Jul 28, 2010 at 10:00:02PM +0200, Dan Carpenter wrote:
> This should be -EBUSY as well. The same for "[PATCH 06/10] ALSA: msnd:
> check request_region() return value"
>
> Another way to write that would be:
>

Gar. I was thinking of request_resource(). request_region() returns a
pointer of course.

But still the return code should probably be -EBUSY. Resource
conflicts are more likely than allocation failures.

regards,
dan carpenter

> - request_region(dev.io, dev.numio, dev.name);
> + err = request_region(dev.io, dev.numio, dev.name);
> + if (err) {
> + free_irq(dev.irq, &dev);
> + return err;
> + }
>
> regards,
> dan carpenter
>
> > + if (request_region(dev.io, dev.numio, dev.name) == NULL) {
> > + free_irq(dev.irq, &dev);
> > + return -ENOMEM;
> > + }

2010-07-29 10:22:13

by Takashi Iwai

Subject: Re: [PATCH 09/10] oss: msnd: check request_region() return value

At Wed, 28 Jul 2010 23:20:31 +0200,
Dan Carpenter wrote:
>
> On Wed, Jul 28, 2010 at 10:00:02PM +0200, Dan Carpenter wrote:
> > This should be -EBUSY as well. The same for "[PATCH 06/10] ALSA: msnd:
> > check request_region() return value"
> >
> > Another way to write that would be:
> >
>
> Gar. I was thinking of request_resource(). request_region() returns a
> pointer of course.
>
> But still the return code should probably be -EBUSY. Resource
> conflicts are more likely than allocation failures.

Agreed.

Kulikov, could you rewrite the patches with -EBUSY?


thanks,

Takashi

>
> regards,
> dan carpenter
>
> > - request_region(dev.io, dev.numio, dev.name);
> > + err = request_region(dev.io, dev.numio, dev.name);
> > + if (err) {
> > + free_irq(dev.irq, &dev);
> > + return err;
> > + }
> >
> > regards,
> > dan carpenter
> >
> > > + if (request_region(dev.io, dev.numio, dev.name) == NULL) {
> > > + free_irq(dev.irq, &dev);
> > > + return -ENOMEM;
> > > + }
>

2010-07-29 10:40:30

by Kulikov Vasiliy

Subject: Re: [PATCH 09/10] oss: msnd: check request_region() return value

On Thu, Jul 29, 2010 at 12:22 +0200, Takashi Iwai wrote:
> At Wed, 28 Jul 2010 23:20:31 +0200,
> Dan Carpenter wrote:
> >
> > On Wed, Jul 28, 2010 at 10:00:02PM +0200, Dan Carpenter wrote:
> > > This should be -EBUSY as well. The same for "[PATCH 06/10] ALSA: msnd:
> > > check request_region() return value"
> > >
> > > Another way to write that would be:
> > >
> >
> > Gar. I was thinking of request_resource(). request_region() returns a
> > pointer of course.
> >
> > But still the return code should probably be -EBUSY. Resource
> > conflicts are more likely than allocation failures.
>
> Agreed.
>
> Kulikov, could you rewrite the patches with -EBUSY?
No problem, patches are coming.

>
>
> thanks,
>
> Takashi
>
> >
> > regards,
> > dan carpenter
> >
> > > - request_region(dev.io, dev.numio, dev.name);
> > > + err = request_region(dev.io, dev.numio, dev.name);
> > > + if (err) {
> > > + free_irq(dev.irq, &dev);
> > > + return err;
> > > + }
> > >
> > > regards,
> > > dan carpenter
> > >
> > > > + if (request_region(dev.io, dev.numio, dev.name) == NULL) {
> > > > + free_irq(dev.irq, &dev);
> > > > + return -ENOMEM;
> > > > + }
> >