re: rbd: enforce parent overlap

re: rbd: enforce parent overlap

[Dan Carpenter]

Hello Alex Elder,

This is a semi-automatic email about new static checker warnings.

The patch 64548e0638af: "rbd: enforce parent overlap" from Apr 21,
2013, leads to the following Smatch complaint:

drivers/block/rbd.c:1449 rbd_osd_read_callback()
	 error: we previously assumed 'img_request' could be null (see line 1448)

drivers/block/rbd.c
  1447			img_request = obj_request->img_request;
  1448			layered = img_request && img_request_layered_test(img_request);
                                  ^^^^^^^^^^^
Old check.
  1449			rbd_dev = img_request->rbd_dev;
                                  ^^^^^^^^^^^^^^^^^^^^
New dereference.

  1450		}
  1451	

regards,
dan carpenter

Re: rbd: enforce parent overlap

[Alex Elder]

On 04/30/2013 02:24 AM, Dan Carpenter wrote:
> Hello Alex Elder,
> 
> This is a semi-automatic email about new static checker warnings.

Cool, I've never used smatch before.  Great to get these
automated warnings.

I looked at this, and this is not a problem.  An earlier
commit, about 10 before that one:
    rbd: always check IMG_DATA flag
implements a comparable check.  Any rbd object request
with the IMG_DATA flag set (obj_request_img_data_test()
returns true) will have a non-null image_request pointer.

I suppose I should have asserted it was non-null, I do
that all over the place...

					-Alex

> The patch 64548e0638af: "rbd: enforce parent overlap" from Apr 21,
> 2013, leads to the following Smatch complaint:
> 
> drivers/block/rbd.c:1449 rbd_osd_read_callback()
> 	 error: we previously assumed 'img_request' could be null (see line 1448)
> 
> drivers/block/rbd.c
>   1447			img_request = obj_request->img_request;
>   1448			layered = img_request && img_request_layered_test(img_request);
>                                   ^^^^^^^^^^^
> Old check.
>   1449			rbd_dev = img_request->rbd_dev;
>                                   ^^^^^^^^^^^^^^^^^^^^
> New dereference.
> 
>   1450		}
>   1451	
> 
> regards,
> dan carpenter
> 

Re: rbd: enforce parent overlap

[Dan Carpenter]

On Tue, Apr 30, 2013 at 06:57:14AM -0500, Alex Elder wrote:
> On 04/30/2013 02:24 AM, Dan Carpenter wrote:
> > Hello Alex Elder,
> > 
> > This is a semi-automatic email about new static checker warnings.
> 
> Cool, I've never used smatch before.  Great to get these
> automated warnings.
> 
> I looked at this, and this is not a problem.  An earlier
> commit, about 10 before that one:
>     rbd: always check IMG_DATA flag
> implements a comparable check.  Any rbd object request
> with the IMG_DATA flag set (obj_request_img_data_test()
> returns true) will have a non-null image_request pointer.
> 
> I suppose I should have asserted it was non-null, I do
> that all over the place...

If Smatch knows a variable is never NULL then it ignores
inconsistent NULL checking but it would be better to just remove the
unneeded check.

regards,
dan carpenter