oss: msnd: check request_region() return value
in [Kernel]
|
Prev: cyber2000fb - 24bpp and 32bpp modes not working
Next: ext4 performance regression 2.6.27-stable versus 2.6.32 and later
From: Dan Carpenter on 28 Jul 2010 16:10 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(a)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; > + } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dan Carpenter on 28 Jul 2010 17:30 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; > > + } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Takashi Iwai on 29 Jul 2010 06:30 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; > > > + } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Vasiliy Kulikov on 29 Jul 2010 06:50
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; > > > > + } > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |