public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 09/10] oss: msnd: check request_region() return value
@ 2010-07-28 16:41 Kulikov Vasiliy
  2010-07-28 20:00 ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Kulikov Vasiliy @ 2010-07-28 16:41 UTC (permalink / raw)
  To: kernel-janitors
  Cc: Andrew Veliath, Jaroslav Kysela, Takashi Iwai, Arnd Bergmann,
	Tejun Heo, alsa-devel, linux-kernel

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

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 09/10] oss: msnd: check request_region() return value
  2010-07-28 16:41 [PATCH 09/10] oss: msnd: check request_region() return value Kulikov Vasiliy
@ 2010-07-28 20:00 ` Dan Carpenter
  2010-07-28 21:20   ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-07-28 20:00 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: kernel-janitors, Andrew Veliath, Jaroslav Kysela, Takashi Iwai,
	Arnd Bergmann, Tejun Heo, alsa-devel, linux-kernel

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 <segooon@gmail.com>
> ---
>  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;
> +	}


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 09/10] oss: msnd: check request_region() return value
  2010-07-28 20:00 ` Dan Carpenter
@ 2010-07-28 21:20   ` Dan Carpenter
  2010-07-29 10:22     ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-07-28 21:20 UTC (permalink / raw)
  To: Kulikov Vasiliy, kernel-janitors, Andrew Veliath, Jaroslav Kysela,
	Takashi Iwai, Arnd Bergmann, Tejun Heo, alsa-devel, linux-kernel

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;
> > +	}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 09/10] oss: msnd: check request_region() return value
  2010-07-28 21:20   ` Dan Carpenter
@ 2010-07-29 10:22     ` Takashi Iwai
  2010-07-29 10:40       ` Vasiliy Kulikov
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2010-07-29 10:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Kulikov Vasiliy, kernel-janitors, Andrew Veliath, Jaroslav Kysela,
	Arnd Bergmann, Tejun Heo, alsa-devel, linux-kernel

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;
> > > +	}
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 09/10] oss: msnd: check request_region() return value
  2010-07-29 10:22     ` Takashi Iwai
@ 2010-07-29 10:40       ` Vasiliy Kulikov
  0 siblings, 0 replies; 5+ messages in thread
From: Vasiliy Kulikov @ 2010-07-29 10:40 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Dan Carpenter, kernel-janitors, Andrew Veliath, Jaroslav Kysela,
	Arnd Bergmann, Tejun Heo, alsa-devel, linux-kernel

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;
> > > > +	}
> > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-07-29 10:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-28 16:41 [PATCH 09/10] oss: msnd: check request_region() return value Kulikov Vasiliy
2010-07-28 20:00 ` Dan Carpenter
2010-07-28 21:20   ` Dan Carpenter
2010-07-29 10:22     ` Takashi Iwai
2010-07-29 10:40       ` Vasiliy Kulikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox