[PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nathan Chancellor]

When building with Clang + -Wtautological-pointer-compare:

drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
address of 'req->queue' equal to a null pointer is always false
[-Wtautological-pointer-compare]
        if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
                             ~~~~~^~~~~    ~~~~
drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
address of 'req->usb_req' equal to a null pointer is always false
[-Wtautological-pointer-compare]
        if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
                                                    ~~~~~^~~~~~~    ~~~~
2 warnings generated.

As it notes, these statements will always evaluate to false so remove
them.

Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
Link: https://github.com/ClangBuiltLinux/linux/issues/749
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

Note: I am not sure if these checks were intended to check if the
contents of these arrays were NULL or if there should be some other
checks in lieu of these; I am not familiar with the USB subsystem to
answer this but I will happily respin the patch if this is not correct.

 drivers/usb/gadget/udc/bdc/bdc_ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index a4d9b5e1e50e..d49c6dc1082d 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -540,7 +540,7 @@ static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
 {
 	struct bdc *bdc = ep->bdc;
 
-	if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
+	if (req == NULL)
 		return;
 
 	dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
-- 
2.23.0

Re: [PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nathan Chancellor]

I know it has been a while but ping?

On Tue, Oct 22, 2019 at 05:20:15PM -0700, Nathan Chancellor wrote:
> When building with Clang + -Wtautological-pointer-compare:
> 
> drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> address of 'req->queue' equal to a null pointer is always false
> [-Wtautological-pointer-compare]
>         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>                              ~~~~~^~~~~    ~~~~
> drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> address of 'req->usb_req' equal to a null pointer is always false
> [-Wtautological-pointer-compare]
>         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>                                                     ~~~~~^~~~~~~    ~~~~
> 2 warnings generated.
> 
> As it notes, these statements will always evaluate to false so remove
> them.
> 
> Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
> Link: https://github.com/ClangBuiltLinux/linux/issues/749
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> Note: I am not sure if these checks were intended to check if the
> contents of these arrays were NULL or if there should be some other
> checks in lieu of these; I am not familiar with the USB subsystem to
> answer this but I will happily respin the patch if this is not correct.
> 
>  drivers/usb/gadget/udc/bdc/bdc_ep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
> index a4d9b5e1e50e..d49c6dc1082d 100644
> --- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
> +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
> @@ -540,7 +540,7 @@ static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
>  {
>  	struct bdc *bdc = ep->bdc;
>  
> -	if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> +	if (req == NULL)
>  		return;
>  
>  	dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
> -- 
> 2.23.0
> 

Re: [PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nick Desaulniers]

On Thu, Feb 20, 2020 at 8:57 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> I know it has been a while but ping?

Sorry! Too many bugs...barely treading water! Send help!

>
> On Tue, Oct 22, 2019 at 05:20:15PM -0700, Nathan Chancellor wrote:
> > When building with Clang + -Wtautological-pointer-compare:
> >
> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> > address of 'req->queue' equal to a null pointer is always false
> > [-Wtautological-pointer-compare]
> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >                              ~~~~~^~~~~    ~~~~
> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> > address of 'req->usb_req' equal to a null pointer is always false
> > [-Wtautological-pointer-compare]
> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >                                                     ~~~~~^~~~~~~    ~~~~
> > 2 warnings generated.
> >
> > As it notes, these statements will always evaluate to false so remove
> > them.

`req` is an instance of a `struct bdc_req` defined in
drivers/usb/gadget/udc/bdc/bdc.h as:
333 struct bdc_req {
334   struct usb_request  usb_req;
335   struct list_head  queue;
336   struct bdc_ep   *ep;
337   /* only one Transfer per request */
338   struct bd_transfer bd_xfr;
339   int epnum;
340 };

So indeed the non-pointer, struct members can never be NULL.

I think the second check that was removed should be
`req->usb_req.complete == NULL`, since otherwise `&req->usb_req` may
be passed to usb_gadget_giveback_request which tries to invoke the
`complete` member as a callback.  There are numerous places in
drivers/usb/gadget/udc/bdc/bdc_ep.c that assign `complete = NULL`.

Can the maintainers clarify?

> >
> > Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/749
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > Note: I am not sure if these checks were intended to check if the
> > contents of these arrays were NULL or if there should be some other
> > checks in lieu of these; I am not familiar with the USB subsystem to
> > answer this but I will happily respin the patch if this is not correct.
> >
> >  drivers/usb/gadget/udc/bdc/bdc_ep.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
> > index a4d9b5e1e50e..d49c6dc1082d 100644
> > --- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
> > +++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
> > @@ -540,7 +540,7 @@ static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
> >  {
> >       struct bdc *bdc = ep->bdc;
> >
> > -     if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> > +     if (req == NULL)
> >               return;
> >
> >       dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
> > --
> > 2.23.0
> >
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200221045740.GA43417%40ubuntu-m2-xlarge-x86.



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nathan Chancellor]

On Mon, Feb 24, 2020 at 01:42:57PM -0800, Nick Desaulniers wrote:
> On Thu, Feb 20, 2020 at 8:57 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > I know it has been a while but ping?
> 
> Sorry! Too many bugs...barely treading water! Send help!
> 
> >
> > On Tue, Oct 22, 2019 at 05:20:15PM -0700, Nathan Chancellor wrote:
> > > When building with Clang + -Wtautological-pointer-compare:
> > >
> > > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> > > address of 'req->queue' equal to a null pointer is always false
> > > [-Wtautological-pointer-compare]
> > >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> > >                              ~~~~~^~~~~    ~~~~
> > > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> > > address of 'req->usb_req' equal to a null pointer is always false
> > > [-Wtautological-pointer-compare]
> > >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> > >                                                     ~~~~~^~~~~~~    ~~~~
> > > 2 warnings generated.
> > >
> > > As it notes, these statements will always evaluate to false so remove
> > > them.
> 
> `req` is an instance of a `struct bdc_req` defined in
> drivers/usb/gadget/udc/bdc/bdc.h as:
> 333 struct bdc_req {
> 334   struct usb_request  usb_req;
> 335   struct list_head  queue;
> 336   struct bdc_ep   *ep;
> 337   /* only one Transfer per request */
> 338   struct bd_transfer bd_xfr;
> 339   int epnum;
> 340 };
> 
> So indeed the non-pointer, struct members can never be NULL.
> 
> I think the second check that was removed should be
> `req->usb_req.complete == NULL`, since otherwise `&req->usb_req` may
> be passed to usb_gadget_giveback_request which tries to invoke the
> `complete` member as a callback.  There are numerous places in
> drivers/usb/gadget/udc/bdc/bdc_ep.c that assign `complete = NULL`.
> 
> Can the maintainers clarify?

$ sed -n 537,555p drivers/usb/gadget/udc/bdc/bdc_ep.c
/* callback to gadget layer when xfr completes */
static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
						int status)
{
	struct bdc *bdc = ep->bdc;

	if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
		return;

	dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
	list_del(&req->queue);
	req->usb_req.status = status;
	usb_gadget_unmap_request(&bdc->gadget, &req->usb_req, ep->dir);
	if (req->usb_req.complete) {
		spin_unlock(&bdc->lock);
		usb_gadget_giveback_request(&ep->usb_ep, &req->usb_req);
		spin_lock(&bdc->lock);
	}
}

It looks like req->usb_req.complete is checked before being passed to
usb_gadget_giveback_request. So the patch as it stands is correct,
unless those checks needed to be something else.

Felipe, could you clarify or pick up this patch if it is correct? This
is one of two warnings that I see for -Wtautological-compare and I want
it turned on for 5.7 and it'd be nice to be warning free, especially
since I sent this patch back in October :/

Cheers,
Nathan

Re: [PATCH] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3354 bytes --]


Hi,

Nathan Chancellor <natechancellor@gmail.com> writes:
> On Mon, Feb 24, 2020 at 01:42:57PM -0800, Nick Desaulniers wrote:
>> On Thu, Feb 20, 2020 at 8:57 PM Nathan Chancellor
>> <natechancellor@gmail.com> wrote:
>> >
>> > I know it has been a while but ping?
>> 
>> Sorry! Too many bugs...barely treading water! Send help!
>> 
>> >
>> > On Tue, Oct 22, 2019 at 05:20:15PM -0700, Nathan Chancellor wrote:
>> > > When building with Clang + -Wtautological-pointer-compare:
>> > >
>> > > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
>> > > address of 'req->queue' equal to a null pointer is always false
>> > > [-Wtautological-pointer-compare]
>> > >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> > >                              ~~~~~^~~~~    ~~~~
>> > > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
>> > > address of 'req->usb_req' equal to a null pointer is always false
>> > > [-Wtautological-pointer-compare]
>> > >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> > >                                                     ~~~~~^~~~~~~    ~~~~
>> > > 2 warnings generated.
>> > >
>> > > As it notes, these statements will always evaluate to false so remove
>> > > them.
>> 
>> `req` is an instance of a `struct bdc_req` defined in
>> drivers/usb/gadget/udc/bdc/bdc.h as:
>> 333 struct bdc_req {
>> 334   struct usb_request  usb_req;
>> 335   struct list_head  queue;
>> 336   struct bdc_ep   *ep;
>> 337   /* only one Transfer per request */
>> 338   struct bd_transfer bd_xfr;
>> 339   int epnum;
>> 340 };
>> 
>> So indeed the non-pointer, struct members can never be NULL.
>> 
>> I think the second check that was removed should be
>> `req->usb_req.complete == NULL`, since otherwise `&req->usb_req` may
>> be passed to usb_gadget_giveback_request which tries to invoke the
>> `complete` member as a callback.  There are numerous places in
>> drivers/usb/gadget/udc/bdc/bdc_ep.c that assign `complete = NULL`.
>> 
>> Can the maintainers clarify?
>
> $ sed -n 537,555p drivers/usb/gadget/udc/bdc/bdc_ep.c
> /* callback to gadget layer when xfr completes */
> static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
> 						int status)
> {
> 	struct bdc *bdc = ep->bdc;
>
> 	if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> 		return;
>
> 	dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
> 	list_del(&req->queue);
> 	req->usb_req.status = status;
> 	usb_gadget_unmap_request(&bdc->gadget, &req->usb_req, ep->dir);
> 	if (req->usb_req.complete) {
> 		spin_unlock(&bdc->lock);
> 		usb_gadget_giveback_request(&ep->usb_ep, &req->usb_req);
> 		spin_lock(&bdc->lock);
> 	}
> }
>
> It looks like req->usb_req.complete is checked before being passed to
> usb_gadget_giveback_request. So the patch as it stands is correct,
> unless those checks needed to be something else.
>
> Felipe, could you clarify or pick up this patch if it is correct? This
> is one of two warnings that I see for -Wtautological-compare and I want
> it turned on for 5.7 and it'd be nice to be warning free, especially
> since I sent this patch back in October :/

hmm, I don't have that patch in my inbox. Could you resend it?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nathan Chancellor]

When building with Clang + -Wtautological-pointer-compare:

drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
address of 'req->queue' equal to a null pointer is always false
[-Wtautological-pointer-compare]
        if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
                             ~~~~~^~~~~    ~~~~
drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
address of 'req->usb_req' equal to a null pointer is always false
[-Wtautological-pointer-compare]
        if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
                                                    ~~~~~^~~~~~~    ~~~~
2 warnings generated.

As it notes, these statements will always evaluate to false so remove
them.

Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
Link: https://github.com/ClangBuiltLinux/linux/issues/749
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc_ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index a4d9b5e1e50e..d49c6dc1082d 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -540,7 +540,7 @@ static void bdc_req_complete(struct bdc_ep *ep, struct bdc_req *req,
 {
 	struct bdc *bdc = ep->bdc;
 
-	if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
+	if (req == NULL)
 		return;
 
 	dev_dbg(bdc->dev, "%s ep:%s status:%d\n", __func__, ep->name, status);
-- 
2.26.0

Re: [PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]


Hi Nathan,

Nathan Chancellor <natechancellor@gmail.com> writes:

> When building with Clang + -Wtautological-pointer-compare:
>
> drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> address of 'req->queue' equal to a null pointer is always false
> [-Wtautological-pointer-compare]
>         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>                              ~~~~~^~~~~    ~~~~
> drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> address of 'req->usb_req' equal to a null pointer is always false
> [-Wtautological-pointer-compare]
>         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>                                                     ~~~~~^~~~~~~    ~~~~
> 2 warnings generated.
>
> As it notes, these statements will always evaluate to false so remove
> them.
>
> Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
> Link: https://github.com/ClangBuiltLinux/linux/issues/749
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

It's now in my queue for v5.8. It doesn't really look like a bug fix, so
it's not going in during v5.7-rc

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Nathan Chancellor]

On Sun, Mar 29, 2020 at 10:43:54AM +0300, Felipe Balbi wrote:
> 
> Hi Nathan,
> 
> Nathan Chancellor <natechancellor@gmail.com> writes:
> 
> > When building with Clang + -Wtautological-pointer-compare:
> >
> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> > address of 'req->queue' equal to a null pointer is always false
> > [-Wtautological-pointer-compare]
> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >                              ~~~~~^~~~~    ~~~~
> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> > address of 'req->usb_req' equal to a null pointer is always false
> > [-Wtautological-pointer-compare]
> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >                                                     ~~~~~^~~~~~~    ~~~~
> > 2 warnings generated.
> >
> > As it notes, these statements will always evaluate to false so remove
> > them.
> >
> > Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/749
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> It's now in my queue for v5.8. It doesn't really look like a bug fix, so
> it's not going in during v5.7-rc
> 
> -- 
> balbi

Thank you for picking it up. It would be nice to see it in 5.7 since
we're enabling this warning and this is one of two outstanding
instances in -next and the other one's patch has been picked up plus the
patch itself is rather benign. Not to mention that I did send this patch
back in October. However, when it is merged into Linus' tree is
ultimately your call so I won't argue as long as it gets there
eventually.

Cheers,
Nathan

Re: [PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1853 bytes --]


Hi,

Nathan Chancellor <natechancellor@gmail.com> writes:
>> > When building with Clang + -Wtautological-pointer-compare:
>> >
>> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
>> > address of 'req->queue' equal to a null pointer is always false
>> > [-Wtautological-pointer-compare]
>> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> >                              ~~~~~^~~~~    ~~~~
>> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
>> > address of 'req->usb_req' equal to a null pointer is always false
>> > [-Wtautological-pointer-compare]
>> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> >                                                     ~~~~~^~~~~~~    ~~~~
>> > 2 warnings generated.
>> >
>> > As it notes, these statements will always evaluate to false so remove
>> > them.
>> >
>> > Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
>> > Link: https://github.com/ClangBuiltLinux/linux/issues/749
>> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> 
>> It's now in my queue for v5.8. It doesn't really look like a bug fix, so
>> it's not going in during v5.7-rc
>> 
>> -- 
>> balbi
>
> Thank you for picking it up. It would be nice to see it in 5.7 since
> we're enabling this warning and this is one of two outstanding
> instances in -next and the other one's patch has been picked up plus the
> patch itself is rather benign. Not to mention that I did send this patch
> back in October. However, when it is merged into Linus' tree is
> ultimately your call so I won't argue as long as it gets there
> eventually.

If Greg's okay with this patch going in during v5.7-rc, I can send it as
a fix, no worries. Greg?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Greg Kroah-Hartman]

On Sun, Mar 29, 2020 at 07:43:52PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Nathan Chancellor <natechancellor@gmail.com> writes:
> >> > When building with Clang + -Wtautological-pointer-compare:
> >> >
> >> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
> >> > address of 'req->queue' equal to a null pointer is always false
> >> > [-Wtautological-pointer-compare]
> >> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >> >                              ~~~~~^~~~~    ~~~~
> >> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
> >> > address of 'req->usb_req' equal to a null pointer is always false
> >> > [-Wtautological-pointer-compare]
> >> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
> >> >                                                     ~~~~~^~~~~~~    ~~~~
> >> > 2 warnings generated.
> >> >
> >> > As it notes, these statements will always evaluate to false so remove
> >> > them.
> >> >
> >> > Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/749
> >> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> >> 
> >> It's now in my queue for v5.8. It doesn't really look like a bug fix, so
> >> it's not going in during v5.7-rc
> >> 
> >> -- 
> >> balbi
> >
> > Thank you for picking it up. It would be nice to see it in 5.7 since
> > we're enabling this warning and this is one of two outstanding
> > instances in -next and the other one's patch has been picked up plus the
> > patch itself is rather benign. Not to mention that I did send this patch
> > back in October. However, when it is merged into Linus' tree is
> > ultimately your call so I won't argue as long as it gets there
> > eventually.
> 
> If Greg's okay with this patch going in during v5.7-rc, I can send it as
> a fix, no worries. Greg?

Yes, clang build warnings fixes are valid fixes for the -rc period, and
I take them into stable where needed as well.

thanks,

greg k-h

Re: [PATCH RESEND] usb: gadget: udc: bdc: Remove unnecessary NULL checks in bdc_req_complete

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

Hi,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> >> > When building with Clang + -Wtautological-pointer-compare:
>> >> >
>> >> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:28: warning: comparison of
>> >> > address of 'req->queue' equal to a null pointer is always false
>> >> > [-Wtautological-pointer-compare]
>> >> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> >> >                              ~~~~~^~~~~    ~~~~
>> >> > drivers/usb/gadget/udc/bdc/bdc_ep.c:543:51: warning: comparison of
>> >> > address of 'req->usb_req' equal to a null pointer is always false
>> >> > [-Wtautological-pointer-compare]
>> >> >         if (req == NULL  || &req->queue == NULL || &req->usb_req == NULL)
>> >> >                                                     ~~~~~^~~~~~~    ~~~~
>> >> > 2 warnings generated.
>> >> >
>> >> > As it notes, these statements will always evaluate to false so remove
>> >> > them.
>> >> >
>> >> > Fixes: efed421a94e6 ("usb: gadget: Add UDC driver for Broadcom USB3.0 device controller IP BDC")
>> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/749
>> >> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> >> 
>> >> It's now in my queue for v5.8. It doesn't really look like a bug fix, so
>> >> it's not going in during v5.7-rc
>> >> 
>> >> -- 
>> >> balbi
>> >
>> > Thank you for picking it up. It would be nice to see it in 5.7 since
>> > we're enabling this warning and this is one of two outstanding
>> > instances in -next and the other one's patch has been picked up plus the
>> > patch itself is rather benign. Not to mention that I did send this patch
>> > back in October. However, when it is merged into Linus' tree is
>> > ultimately your call so I won't argue as long as it gets there
>> > eventually.
>> 
>> If Greg's okay with this patch going in during v5.7-rc, I can send it as
>> a fix, no worries. Greg?
>
> Yes, clang build warnings fixes are valid fixes for the -rc period, and
> I take them into stable where needed as well.

Thanks for confirming, Greg. I'll move the commit to my testing/fixes

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]