Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Julia Lawall]

On Wed, 23 Nov 2016, Fengguang Wu wrote:

> On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote:
> > On Mon, 21 Nov 2016, Benjamin Tissoires wrote:
> >
> > > > Generated by: scripts/coccinelle/misc/noderef.cocci
> > > >
> > > > CC: Benjamin Tissoires 
> > > > Signed-off-by: Fengguang Wu 
> > > > ---
> > > >
> > > >  hid-lg.c |6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > --- a/drivers/hid/hid-lg.c
> > > > +++ b/drivers/hid/hid-lg.c
> > > > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
> > > > buf[1] = 0xB2;
> > > > get_random_bytes([2], 2);
> > > >
> > > > -   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > > sizeof(buf),
> > > > -   HID_FEATURE_REPORT,
> > > HID_REQ_SET_REPORT);
> > > > +   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > > > +sizeof(*buf),
> > >
> > > This is wrong. I messed up and should have used "sizeof(cbuf)", but the
> > > coccinelle script failed at detecting the correct solution (I guess it
> > > couldn't).
> >
> > Fengguang, is there anything that could be done to improve this?
>
> CC Julie and Gilles. I'm not sure if the coccinelle script could be
> made that smart. :)

Thanks for forwarding.  From looking at the code snippet, I have the
impression that if it were possible, it would require an interprocedural
analysis, and the cost would outweigh the benefit.  Basically, I don't see
any cbuf nearby.

julia


> > > Jiri, do you want me to send a v2 of the series or will you just amend
> > > the patch while applying?
> >
> > I'll fix that up, no worries. Thanks,
> >
> > --
> > Jiri Kosina
> > SUSE Labs
>

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Julia Lawall]

On Wed, 23 Nov 2016, Fengguang Wu wrote:

> On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote:
> > On Mon, 21 Nov 2016, Benjamin Tissoires wrote:
> >
> > > > Generated by: scripts/coccinelle/misc/noderef.cocci
> > > >
> > > > CC: Benjamin Tissoires 
> > > > Signed-off-by: Fengguang Wu 
> > > > ---
> > > >
> > > >  hid-lg.c |6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > --- a/drivers/hid/hid-lg.c
> > > > +++ b/drivers/hid/hid-lg.c
> > > > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
> > > > buf[1] = 0xB2;
> > > > get_random_bytes([2], 2);
> > > >
> > > > -   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > > sizeof(buf),
> > > > -   HID_FEATURE_REPORT,
> > > HID_REQ_SET_REPORT);
> > > > +   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > > > +sizeof(*buf),
> > >
> > > This is wrong. I messed up and should have used "sizeof(cbuf)", but the
> > > coccinelle script failed at detecting the correct solution (I guess it
> > > couldn't).
> >
> > Fengguang, is there anything that could be done to improve this?
>
> CC Julie and Gilles. I'm not sure if the coccinelle script could be
> made that smart. :)

Thanks for forwarding.  From looking at the code snippet, I have the
impression that if it were possible, it would require an interprocedural
analysis, and the cost would outweigh the benefit.  Basically, I don't see
any cbuf nearby.

julia


> > > Jiri, do you want me to send a v2 of the series or will you just amend
> > > the patch while applying?
> >
> > I'll fix that up, no worries. Thanks,
> >
> > --
> > Jiri Kosina
> > SUSE Labs
>

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Fengguang Wu]

On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote:

On Mon, 21 Nov 2016, Benjamin Tissoires wrote:


> Generated by: scripts/coccinelle/misc/noderef.cocci
>
> CC: Benjamin Tissoires 
> Signed-off-by: Fengguang Wu 
> ---
>
>  hid-lg.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
>buf[1] = 0xB2;
>get_random_bytes([2], 2);
>
> -  ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> -  HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +  ret = hid_hw_raw_request(hdev, buf[0], buf,
> +   sizeof(*buf),

This is wrong. I messed up and should have used "sizeof(cbuf)", but the
coccinelle script failed at detecting the correct solution (I guess it
couldn't).


Fengguang, is there anything that could be done to improve this?


CC Julie and Gilles. I'm not sure if the coccinelle script could be
made that smart. :)


Jiri, do you want me to send a v2 of the series or will you just amend
the patch while applying?


I'll fix that up, no worries. Thanks,

--
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Fengguang Wu]

On Tue, Nov 22, 2016 at 11:44:34AM +0100, Jiri Kosina wrote:

On Mon, 21 Nov 2016, Benjamin Tissoires wrote:


> Generated by: scripts/coccinelle/misc/noderef.cocci
>
> CC: Benjamin Tissoires 
> Signed-off-by: Fengguang Wu 
> ---
>
>  hid-lg.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
>buf[1] = 0xB2;
>get_random_bytes([2], 2);
>
> -  ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> -  HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +  ret = hid_hw_raw_request(hdev, buf[0], buf,
> +   sizeof(*buf),

This is wrong. I messed up and should have used "sizeof(cbuf)", but the
coccinelle script failed at detecting the correct solution (I guess it
couldn't).


Fengguang, is there anything that could be done to improve this?


CC Julie and Gilles. I'm not sure if the coccinelle script could be
made that smart. :)


Jiri, do you want me to send a v2 of the series or will you just amend
the patch while applying?


I'll fix that up, no worries. Thanks,

--
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Jiri Kosina]

On Mon, 21 Nov 2016, Benjamin Tissoires wrote:

> > Generated by: scripts/coccinelle/misc/noderef.cocci
> > 
> > CC: Benjamin Tissoires 
> > Signed-off-by: Fengguang Wu 
> > ---
> > 
> >  hid-lg.c |6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > --- a/drivers/hid/hid-lg.c
> > +++ b/drivers/hid/hid-lg.c
> > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
> > buf[1] = 0xB2;
> > get_random_bytes([2], 2);
> >  
> > -   ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> > -   HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> > +   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > +sizeof(*buf),
> 
> This is wrong. I messed up and should have used "sizeof(cbuf)", but the
> coccinelle script failed at detecting the correct solution (I guess it
> couldn't).

Fengguang, is there anything that could be done to improve this?

> Jiri, do you want me to send a v2 of the series or will you just amend 
> the patch while applying?

I'll fix that up, no worries. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Jiri Kosina]

On Mon, 21 Nov 2016, Benjamin Tissoires wrote:

> > Generated by: scripts/coccinelle/misc/noderef.cocci
> > 
> > CC: Benjamin Tissoires 
> > Signed-off-by: Fengguang Wu 
> > ---
> > 
> >  hid-lg.c |6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > --- a/drivers/hid/hid-lg.c
> > +++ b/drivers/hid/hid-lg.c
> > @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
> > buf[1] = 0xB2;
> > get_random_bytes([2], 2);
> >  
> > -   ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> > -   HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> > +   ret = hid_hw_raw_request(hdev, buf[0], buf,
> > +sizeof(*buf),
> 
> This is wrong. I messed up and should have used "sizeof(cbuf)", but the
> coccinelle script failed at detecting the correct solution (I guess it
> couldn't).

Fengguang, is there anything that could be done to improve this?

> Jiri, do you want me to send a v2 of the series or will you just amend 
> the patch while applying?

I'll fix that up, no worries. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Benjamin Tissoires]

On Nov 21 2016 or thereabouts, kbuild test robot wrote:
> drivers/hid/hid-lg.c:780:47-53: ERROR: application of sizeof to pointer
> 
>  sizeof when applied to a pointer typed expression gives the size of
>  the pointer
> 
> Generated by: scripts/coccinelle/misc/noderef.cocci
> 
> CC: Benjamin Tissoires 
> Signed-off-by: Fengguang Wu 
> ---
> 
>  hid-lg.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
>   buf[1] = 0xB2;
>   get_random_bytes([2], 2);
>  
> - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> - HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + ret = hid_hw_raw_request(hdev, buf[0], buf,
> +  sizeof(*buf),

This is wrong. I messed up and should have used "sizeof(cbuf)", but the
coccinelle script failed at detecting the correct solution (I guess it
couldn't).

Jiri, do you want me to send a v2 of the series or will you just amend
the patch while applying?

Cheers,
Benjamin

> +  HID_FEATURE_REPORT,
> +  HID_REQ_SET_REPORT);
>   }
>   kfree(buf);
>   }

Re: [PATCH] HID: lg: fix noderef.cocci warnings

[Benjamin Tissoires]

On Nov 21 2016 or thereabouts, kbuild test robot wrote:
> drivers/hid/hid-lg.c:780:47-53: ERROR: application of sizeof to pointer
> 
>  sizeof when applied to a pointer typed expression gives the size of
>  the pointer
> 
> Generated by: scripts/coccinelle/misc/noderef.cocci
> 
> CC: Benjamin Tissoires 
> Signed-off-by: Fengguang Wu 
> ---
> 
>  hid-lg.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -777,8 +777,10 @@ static int lg_probe(struct hid_device *h
>   buf[1] = 0xB2;
>   get_random_bytes([2], 2);
>  
> - ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> - HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + ret = hid_hw_raw_request(hdev, buf[0], buf,
> +  sizeof(*buf),

This is wrong. I messed up and should have used "sizeof(cbuf)", but the
coccinelle script failed at detecting the correct solution (I guess it
couldn't).

Jiri, do you want me to send a v2 of the series or will you just amend
the patch while applying?

Cheers,
Benjamin

> +  HID_FEATURE_REPORT,
> +  HID_REQ_SET_REPORT);
>   }
>   kfree(buf);
>   }