On Thu, Feb 20, 2020 at 8:57 PM Nathan Chancellor
[off-list ref] 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:

quoted

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?

quoted

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 <redacted>
---

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