Re: [PATCH 1/2] media: pxa_camera: don't deferenciate a NULL pointer

2018-12-07 Thread Sakari Ailus
Hi Mauro,

On Fri, Dec 07, 2018 at 08:07:54AM -0500, Mauro Carvalho Chehab wrote:
> As warned by smatch:
>   drivers/media/platform/pxa_camera.c:2400 pxa_camera_probe() error: we 
> previously assumed 'pcdev->pdata' could be null (see line 2397)
> 
> It would be possible that neither DT nor platform data would be
> provided. This is a Kernel bug, so warn about that and bail.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/pxa_camera.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/pxa_camera.c 
> b/drivers/media/platform/pxa_camera.c
> index 5f930560eb30..f91f8fd424c4 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2396,6 +2396,9 @@ static int pxa_camera_probe(struct platform_device 
> *pdev)
>   pcdev->pdata = pdev->dev.platform_data;
>   if (pdev->dev.of_node && !pcdev->pdata) {
>   err = pxa_camera_pdata_from_dt(>dev, pcdev, >asd);
> + } else if (!pcdev->pdata) {

This fixes the issue, but the current checks remain a bit odd.

The driver seems to prefer platform data over OF. I wonder if that's
intentional or not.

In that case, I'd roughly write this as:

if (pcdev->pdata) {
...;
} else if (pdev->dev.of_node) {
...;
} else {
return -ENODEV;
}

I'm not sure WARN_ON(1) is necessary. A lot of drivers simply do it this
way without WARN_ON().

> + WARN_ON(1);
> + return -ENODEV;
>   } else {
>   pcdev->platform_flags = pcdev->pdata->flags;
>   pcdev->mclk = pcdev->pdata->mclk_10khz * 1;

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


[PATCH 1/2] media: pxa_camera: don't deferenciate a NULL pointer

2018-12-07 Thread Mauro Carvalho Chehab
As warned by smatch:
drivers/media/platform/pxa_camera.c:2400 pxa_camera_probe() error: we 
previously assumed 'pcdev->pdata' could be null (see line 2397)

It would be possible that neither DT nor platform data would be
provided. This is a Kernel bug, so warn about that and bail.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/pxa_camera.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/pxa_camera.c 
b/drivers/media/platform/pxa_camera.c
index 5f930560eb30..f91f8fd424c4 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2396,6 +2396,9 @@ static int pxa_camera_probe(struct platform_device *pdev)
pcdev->pdata = pdev->dev.platform_data;
if (pdev->dev.of_node && !pcdev->pdata) {
err = pxa_camera_pdata_from_dt(>dev, pcdev, >asd);
+   } else if (!pcdev->pdata) {
+   WARN_ON(1);
+   return -ENODEV;
} else {
pcdev->platform_flags = pcdev->pdata->flags;
pcdev->mclk = pcdev->pdata->mclk_10khz * 1;
-- 
2.19.2