RE: [bug report] media: staging/intel-ipu3: Add imgu top level pci device driver

["Zhi, Yong" <yong.zhi@intel.com>]

Cc Tianshu and others.

Hi, Dan,

Thanks a lot for the code review.

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, January 4, 2019 6:29 AM
> To: Zhi, Yong <yong.zhi@intel.com>
> Cc: linux-media@vger.kernel.org
> Subject: [bug report] media: staging/intel-ipu3: Add imgu top level pci device
> driver
> 
> Hello Yong Zhi,
> 
> The patch 7fc7af649ca7: "media: staging/intel-ipu3: Add imgu top level pci
> device driver" from Dec 6, 2018, leads to the following static checker warning:
> 
> 	drivers/staging/media/ipu3/ipu3.c:493 imgu_isr_threaded()
> 	warn: 'b' is an error pointer or valid
> 
> drivers/staging/media/ipu3/ipu3.c
>     472 static irqreturn_t imgu_isr_threaded(int irq, void *imgu_ptr)
>     473 {
>     474 	struct imgu_device *imgu = imgu_ptr;
>     475 	struct imgu_media_pipe *imgu_pipe;
>     476 	int p;
>     477
>     478 	/* Dequeue / queue buffers */
>     479 	do {
>     480 		u64 ns = ktime_get_ns();
>     481 		struct ipu3_css_buffer *b;
>     482 		struct imgu_buffer *buf = NULL;
>     483 		unsigned int node, pipe;
>     484 		bool dummy;
>     485
>     486 		do {
>     487 			mutex_lock(&imgu->lock);
>     488 			b = ipu3_css_buf_dequeue(&imgu->css);
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ipu3_css_buf_dequeue() doesn't return NULL.
> 
>     489 			mutex_unlock(&imgu->lock);
>     490 		} while (PTR_ERR(b) == -EAGAIN);
>     491
>     492 		if (IS_ERR_OR_NULL(b)) {
>                             ^^^^^^^^^^^^^^^^^
> --> 493 			if (!b || PTR_ERR(b) == -EBUSY)	/* All done */
>                                     ^^
> When a function returns both NULL and error pointers, then NULL is
> considered a special case of success.  Like perhaps you request a feature, but
> that feature isn't enabled in the config.  It's fine, because the user *chose* to
> turn off the feature, so it's not an error but we also don't have a valid pointer
> we can use.
> 
> It looks like you were probably trying to do something like that but you
> missed part of the commit?  Otherwise we should delete the dead code.
> 

Ack, with recent code changes the NULL check becomes useless, thanks for catching this.

>     494 				break;
>     495 			dev_err(&imgu->pci_dev->dev,
>     496 				"failed to dequeue buffers (%ld)\n",
>     497 				PTR_ERR(b));
>     498 			break;
>     499 		}
>     500
> 
> regards,
> dan carpenter