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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 27, 2014 at 02:52:34PM -0800, Greg KH wrote:
> On Wed, Feb 26, 2014 at 08:53:39PM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
> > 
> > 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.carpenter@xxxxxxxxxx>
> > Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
> > ---
> > 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?

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.

You're quite right:

        for (i = 0; i < DMFC_NUM_CHANNELS; i++) {
                if (priv->channels[i].slots > 0)
                        ipu_dmfc_setup_channel(&priv->channels[i],

it can never be NULL here, and the only other place this is called is:

ipu_dmfc_alloc_bandwidth()
{
...
        /* For the MEM_BG channel, first try to allocate twice the slots */
        if (dmfc->data->ipu_channel == IPUV3_CHANNEL_MEM_BG_SYNC)
                segment = dmfc_find_slots(priv, slots * 2);
        else if (slots < 2)
                /* Always allocate at least 128*4 bytes (2 slots) */
                slots = 2;
...
        ipu_dmfc_setup_channel(dmfc, slots, segment, burstsize);

So it can't be NULL here either, and, therefore, the test in
ipu_dmfc_setup_channel() should be deleted as it serves no useful
purpose.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux