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 <[email protected]>
---
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;
--
2.5.0
Em Thu, 14 Apr 2016 10:31:20 -0600
Shuah Khan <[email protected]> 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 <[email protected]>
> ---
> 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;
--
Thanks,
Mauro
On 04/14/2016 03:08 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 14 Apr 2016 10:31:20 -0600
> Shuah Khan <[email protected]> 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
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America(Silicon Valley)
[email protected] | (970) 217-8978
Em Thu, 14 Apr 2016 16:18:17 -0600
Shuah Khan <[email protected]> escreveu:
> On 04/14/2016 03:08 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 14 Apr 2016 10:31:20 -0600
> > Shuah Khan <[email protected]> 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
>
>
--
Thanks,
Mauro