Re: [PATCH v4 1/6] imx-drm: ipu-dmfc: Check 'dmfc' pointer first

2014-02-27 Thread Greg KH 
On Wed, Feb 26, 2014 at 08:53:39PM -0300, Fabio Estevam wrote:
 From: Fabio Estevam fabio.este...@freescale.com
 
 Fix the following static checker warning:
 
 drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c:164 ipu_dmfc_setup_channel() warn: 
 variable dereferenced before check 'dmfc' (see line 157)
 
 Reported-by: Dan Carpenter dan.carpen...@oracle.com
 Signed-off-by: Fabio Estevam fabio.este...@freescale.com
 ---
 Changes since v3:
 - None
 Changes since v2:
 - None
 Changes since v1:
 - Check 'dmfc' prior to setting the priv pointer
 
  drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c 
 b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
 index 98070dd..ce152d9 100644
 --- a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
 +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
 @@ -154,16 +154,18 @@ EXPORT_SYMBOL_GPL(ipu_dmfc_disable_channel);
  static int ipu_dmfc_setup_channel(struct dmfc_channel *dmfc, int slots,
   int segment, int burstsize)
  {
 - struct ipu_dmfc_priv *priv = dmfc-priv;
 + struct ipu_dmfc_priv *priv;
   u32 val, field;
  
 + if (!dmfc)
 + return -EINVAL;

How can dmfc ever be NULL?  You control what values you pass to it, why
check it again?

Have you hit this problem in the wild?  If so, shouldn't the root
cause be fixed and not papered over here?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/6] imx-drm: ipu-dmfc: Check 'dmfc' pointer first

2014-02-27 Thread Dan Carpenter 
On Thu, Feb 27, 2014 at 11:54:23PM +, Russell King - ARM Linux wrote:
 On Thu, Feb 27, 2014 at 02:52:34PM -0800, Greg KH wrote:
  
  How can dmfc ever be NULL?  You control what values you pass to it, why
  check it again?
 
 Indeed.  This is actually a case where the static checker warning is
 ambiguous.  What's triggered it is the dmfc test for NULL after it's
 been used.  It doesn't necessarily mean that the test should be moved
 earlier.
 

Btw, this is another case where Smatch wouldn't have generated a warning
if I had had the cross function database set up.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/6] imx-drm: ipu-dmfc: Check 'dmfc' pointer first

2014-02-26 Thread Fabio Estevam 
From: Fabio Estevam fabio.este...@freescale.com

Fix the following static checker warning:

drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c:164 ipu_dmfc_setup_channel() warn: 
variable dereferenced before check 'dmfc' (see line 157)

Reported-by: Dan Carpenter dan.carpen...@oracle.com
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Changes since v3:
- None
Changes since v2:
- None
Changes since v1:
- Check 'dmfc' prior to setting the priv pointer

 drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c 
b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
index 98070dd..ce152d9 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
@@ -154,16 +154,18 @@ EXPORT_SYMBOL_GPL(ipu_dmfc_disable_channel);
 static int ipu_dmfc_setup_channel(struct dmfc_channel *dmfc, int slots,
int segment, int burstsize)
 {
-   struct ipu_dmfc_priv *priv = dmfc-priv;
+   struct ipu_dmfc_priv *priv;
u32 val, field;
 
+   if (!dmfc)
+   return -EINVAL;
+
+   priv = dmfc-priv;
+
dev_dbg(priv-dev,
dmfc: using %d slots starting from segment %d for IPU 
channel %d\n,
slots, segment, dmfc-data-ipu_channel);
 
-   if (!dmfc)
-   return -EINVAL;
-
switch (slots) {
case 1:
field = DMFC_FIFO_SIZE_64;
-- 
1.8.1.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel