Re: [PATCH] media: vsp1: cleanup a false positive warning

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Mauro,

Thank you for the patch.

On Friday, 4 May 2018 15:13:58 EEST Mauro Carvalho Chehab wrote:
> With the new vsp1 code changes introduced by changeset
> f81f9adc4ee1 ("media: v4l: vsp1: Assign BRU and BRS to pipelines
> dynamically"), smatch complains with:
> 	drivers/media/platform/vsp1/vsp1_drm.c:262 vsp1_du_pipeline_setup_bru()
> error: we previously assumed 'pipe->bru' could be null (see line 180)
> 
> This is a false positive, as, if pipe->bru is NULL, the brx
> var will be different, with ends by calling a code that will
> set pipe->bru to another value.
> 
> Yet, cleaning this false positive is as easy as adding an explicit
> check if pipe->bru is NULL.

It's not very difficult indeed, but it really is a false positive. I think the 
proposed change decreases readability, the condition currently reads as "if 
(new brx != old brx)", why does smatch even flag that as an error ?

> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  drivers/media/platform/vsp1/vsp1_drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index 095dc48aa25a..cb6b60843400
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -185,7 +185,7 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device
> *vsp1, brx = &vsp1->brs->entity;
> 
>  	/* Switch BRx if needed. */
> -	if (brx != pipe->brx) {
> +	if (brx != pipe->brx || !pipe->brx) {
>  		struct vsp1_entity *released_brx = NULL;
> 
>  		/* Release our BRx if we have one. */

-- 
Regards,

Laurent Pinchart