From: Alan Stern <stern@rowland.harvard.edu>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Felipe Balbi <balbi@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Tatyana Brokhman <tlinder@codeaurora.org>,
Felipe Balbi <balbi@ti.com>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: gadget: dummy: fix nonsensical comparisons
Date: Tue, 5 Sep 2017 10:30:38 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1709051027400.1513-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <20170905075720.771242-1-arnd@arndb.de>
On Tue, 5 Sep 2017, Arnd Bergmann wrote:
> gcc-8 points out two comparisons that are clearly bogus
> and almost certainly not what the author intended to write:
>
> drivers/usb/gadget/udc/dummy_hcd.c: In function 'set_link_state_by_speed':
> drivers/usb/gadget/udc/dummy_hcd.c:379:31: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]
> USB_PORT_STAT_ENABLE) == 1 &&
> ^~
> drivers/usb/gadget/udc/dummy_hcd.c:381:25: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]
> USB_SS_PORT_LS_U0) == 1 &&
> ^~
>
> I looked at the code for a bit and came up with a change that makes
> it look like what the author probably meant here. This makes it
> look reasonable to me and to gcc, shutting up the warning.
>
> It does of course change behavior as the two conditions are actually
> evaluated rather than being hardcoded to false, and I have made no
> attempt at verifying that the changed logic makes sense in the context
> of a USB HCD, so that part needs to be reviewed carefully.
>
> Fixes: 1cd8fd2887e1 ("usb: gadget: dummy_hcd: add SuperSpeed support")
> Cc: Tatyana Brokhman <tlinder@codeaurora.org>
> Cc: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index a030d7923d7d..54e8e37d2bc8 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -376,10 +376,10 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd)
> dum_hcd->port_status |=
> (USB_PORT_STAT_C_CONNECTION << 16);
> if ((dum_hcd->port_status &
> - USB_PORT_STAT_ENABLE) == 1 &&
> - (dum_hcd->port_status &
> - USB_SS_PORT_LS_U0) == 1 &&
> - dum_hcd->rh_state != DUMMY_RH_SUSPENDED)
> + USB_PORT_STAT_ENABLE) == USB_PORT_STAT_ENABLE &&
This test can simply become (dum_hcd->port_status & USB_PORT_STAT_ENABLE),
since the latter is a single bit. If you don't mind making its form
different from the forms of the other two tests.
Alan Stern
> + (dum_hcd->port_status &
> + USB_PORT_STAT_LINK_STATE) == USB_SS_PORT_LS_U0 &&
> + dum_hcd->rh_state != DUMMY_RH_SUSPENDED)
> dum_hcd->active = 1;
> }
> } else {
>
prev parent reply other threads:[~2017-09-05 14:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-05 7:56 [PATCH] usb: gadget: dummy: fix nonsensical comparisons Arnd Bergmann
2017-09-05 14:30 ` Alan Stern [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.44L0.1709051027400.1513-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=arnd@arndb.de \
--cc=balbi@kernel.org \
--cc=balbi@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=tlinder@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).