diff mbox series

usb: xhci: fix incorrect memset()

Message ID 20171211112806.3415781-1-arnd@arndb.de
State New
Headers show
Series usb: xhci: fix incorrect memset() | expand

Commit Message

Arnd Bergmann Dec. 11, 2017, 11:27 a.m. UTC
gcc-8 warnings about the new driver using a memset with a bogus length:

drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':
drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

It looks like the author meant to use sizeof() rather than ARRAY_SIZE()
here, so use that.

Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/usb/host/xhci-dbgcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.0

Comments

Mathias Nyman Dec. 11, 2017, 12:59 p.m. UTC | #1
On 11.12.2017 13:27, Arnd Bergmann wrote:
> gcc-8 warnings about the new driver using a memset with a bogus length:

> 

> drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':

> drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

> 

> It looks like the author meant to use sizeof() rather than ARRAY_SIZE()

> here, so use that.

> 

> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---


Another patch to fix the same thing was sent earlier as a follow up to the original series.
https://marc.info/?l=linux-usb&m=151298133524873&w=2

But your patch includes the Fixes line with the commit id, which is nice.

Both fix the problem, It doesn't matter for me which one gets applied

Thanks
-Mathias
Greg KH Dec. 11, 2017, 5:01 p.m. UTC | #2
On Mon, Dec 11, 2017 at 02:59:13PM +0200, Mathias Nyman wrote:
> On 11.12.2017 13:27, Arnd Bergmann wrote:

> > gcc-8 warnings about the new driver using a memset with a bogus length:

> > 

> > drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':

> > drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

> > 

> > It looks like the author meant to use sizeof() rather than ARRAY_SIZE()

> > here, so use that.

> > 

> > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> > ---

> 

> Another patch to fix the same thing was sent earlier as a follow up to the original series.

> https://marc.info/?l=linux-usb&m=151298133524873&w=2

> 

> But your patch includes the Fixes line with the commit id, which is nice.

> 

> Both fix the problem, It doesn't matter for me which one gets applied


They fix it in different ways, which is correct?

I'm guessing you tested yours?  And Arnd probably doesn't have the
hardware to do so?

thanks,

greg k-h
Arnd Bergmann Dec. 11, 2017, 5:05 p.m. UTC | #3
On Mon, Dec 11, 2017 at 6:01 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Dec 11, 2017 at 02:59:13PM +0200, Mathias Nyman wrote:

>> On 11.12.2017 13:27, Arnd Bergmann wrote:

>> > gcc-8 warnings about the new driver using a memset with a bogus length:

>> >

>> > drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':

>> > drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

>> >

>> > It looks like the author meant to use sizeof() rather than ARRAY_SIZE()

>> > here, so use that.

>> >

>> > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")

>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>> > ---

>>

>> Another patch to fix the same thing was sent earlier as a follow up to the original series.

>> https://marc.info/?l=linux-usb&m=151298133524873&w=2

>>

>> But your patch includes the Fixes line with the commit id, which is nice.

>>

>> Both fix the problem, It doesn't matter for me which one gets applied

>

> They fix it in different ways, which is correct?


Both are correct, 'sizeof(dbc->eps)' is the same as 'sizeof(struct
dbc_ep) * ARRAY_SIZE(dbc->eps)'.

> And Arnd probably doesn't have the hardware to do so?


Right.

      Arnd
Mathias Nyman Dec. 12, 2017, 10:47 a.m. UTC | #4
On 11.12.2017 19:05, Arnd Bergmann wrote:
> On Mon, Dec 11, 2017 at 6:01 PM, Greg Kroah-Hartman

> <gregkh@linuxfoundation.org> wrote:

>> On Mon, Dec 11, 2017 at 02:59:13PM +0200, Mathias Nyman wrote:

>>> On 11.12.2017 13:27, Arnd Bergmann wrote:

>>>> gcc-8 warnings about the new driver using a memset with a bogus length:

>>>>

>>>> drivers/usb/host/xhci-dbgcap.c: In function 'xhci_dbc_eps_exit':

>>>> drivers/usb/host/xhci-dbgcap.c:369:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

>>>>

>>>> It looks like the author meant to use sizeof() rather than ARRAY_SIZE()

>>>> here, so use that.

>>>>

>>>> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")

>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>>>> ---

>>>

>>> Another patch to fix the same thing was sent earlier as a follow up to the original series.

>>> https://marc.info/?l=linux-usb&m=151298133524873&w=2

>>>

>>> But your patch includes the Fixes line with the commit id, which is nice.

>>>

>>> Both fix the problem, It doesn't matter for me which one gets applied

>>

>> They fix it in different ways, which is correct?

> 

> Both are correct, 'sizeof(dbc->eps)' is the same as 'sizeof(struct

> dbc_ep) * ARRAY_SIZE(dbc->eps)'.

> 

>> And Arnd probably doesn't have the hardware to do so?

> 

> Right.

> 


Just to test and confirm boyh work I added a couple printk messages after memset()
showing bytes zeroed and method used:

# echo enabled > dbc
# cat dbc
enabled
# [  219.050277] xhci_hcd 0000:00:14.0: DbC connected
[  219.307250] xhci_hcd 0000:00:14.0: DbC configured
[  219.313601] xhci_hcd 0000:00:14.0: DbC now attached to /dev/ttyDBC0

# echo disabled > dbc
[  255.745590] xhci_hcd 0000:00:14.0: set 80 bytes to zero sizeof(struct dbc_ep) * ARRAY_SIZE(dbc->eps)
[  255.755970] xhci_hcd 0000:00:14.0: set 80 bytes to zero sizeof(dbc->eps)

-Mathias
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index 671e5023e683..1e535bd2be01 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -366,7 +366,7 @@  static void xhci_dbc_eps_exit(struct xhci_hcd *xhci)
 {
 	struct xhci_dbc		*dbc = xhci->dbc;
 
-	memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
+	memset(dbc->eps, 0, sizeof(dbc->eps));
 }
 
 static int xhci_dbc_mem_init(struct xhci_hcd *xhci, gfp_t flags)