| Message ID | 1460651480-6935-1-git-send-email-shuahkh@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Thu, 14 Apr 2016 10:31:20 -0600 Shuah Khan <shuahkh@osg.samsung.com> escreveu: > media_dev alloc error path does kfree when alloc fails. Fix it to not call > kfree when media_dev alloc fails. No need. kfree(NULL) is OK. Adding a label inside a conditional block is ugly. > > Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com> > --- > drivers/media/pci/saa7134/saa7134-core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c > index c0e1780..eab2684 100644 > --- a/drivers/media/pci/saa7134/saa7134-core.c > +++ b/drivers/media/pci/saa7134/saa7134-core.c > @@ -1046,7 +1046,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev, > dev->media_dev = kzalloc(sizeof(*dev->media_dev), GFP_KERNEL); > if (!dev->media_dev) { > err = -ENOMEM; > - goto fail0; > + goto media_dev_alloc_fail; > } > media_device_pci_init(dev->media_dev, pci_dev, dev->name); > dev->v4l2_dev.mdev = dev->media_dev; > @@ -1309,6 +1309,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev, > fail0: > #ifdef CONFIG_MEDIA_CONTROLLER > kfree(dev->media_dev); > + media_dev_alloc_fail: > #endif > kfree(dev); > return err;
On 04/14/2016 03:08 PM, Mauro Carvalho Chehab wrote: > Em Thu, 14 Apr 2016 10:31:20 -0600 > Shuah Khan <shuahkh@osg.samsung.com> escreveu: > >> media_dev alloc error path does kfree when alloc fails. Fix it to not call >> kfree when media_dev alloc fails. > > No need. kfree(NULL) is OK. Agreed. > > Adding a label inside a conditional block is ugly. In this case, if label is in normal path, we will see defined, but not used warnings when condition isn't defined. We seem to have many such cases for CONFIG_MEDIA_CONTROLLER :( thanks, -- Shuah
Em Thu, 14 Apr 2016 16:18:17 -0600 Shuah Khan <shuah.kh@samsung.com> escreveu: > On 04/14/2016 03:08 PM, Mauro Carvalho Chehab wrote: > > Em Thu, 14 Apr 2016 10:31:20 -0600 > > Shuah Khan <shuahkh@osg.samsung.com> escreveu: > > > >> media_dev alloc error path does kfree when alloc fails. Fix it to not call > >> kfree when media_dev alloc fails. > > > > No need. kfree(NULL) is OK. > > Agreed. > > > > > Adding a label inside a conditional block is ugly. > > In this case, if label is in normal path, we will see defined, but not > used warnings when condition isn't defined. True, but we don't need a label here, as kfree() can be called with a null pointer. > We seem to have many such > cases for CONFIG_MEDIA_CONTROLLER :( We may try to address those media-controller dependent code latter on. I have some ideas of adding some macros and helper functions to allow getting rid of those ifdefs and not add extra code if !MEDIA_CONTROLLER, but the better seems to first add MC support to ALSA and make the enable/disable functions generic, and then cleanup the code to remove those ifdefs. > > thanks, > -- Shuah > >
diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c index c0e1780..eab2684 100644 --- a/drivers/media/pci/saa7134/saa7134-core.c +++ b/drivers/media/pci/saa7134/saa7134-core.c @@ -1046,7 +1046,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev, dev->media_dev = kzalloc(sizeof(*dev->media_dev), GFP_KERNEL); if (!dev->media_dev) { err = -ENOMEM; - goto fail0; + goto media_dev_alloc_fail; } media_device_pci_init(dev->media_dev, pci_dev, dev->name); dev->v4l2_dev.mdev = dev->media_dev; @@ -1309,6 +1309,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev, fail0: #ifdef CONFIG_MEDIA_CONTROLLER kfree(dev->media_dev); + media_dev_alloc_fail: #endif kfree(dev); return err;
media_dev alloc error path does kfree when alloc fails. Fix it to not call kfree when media_dev alloc fails. Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com> --- drivers/media/pci/saa7134/saa7134-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)