* [PATCH] wusb: use correct format characters
@ 2019-02-28 11:52 Louis Taylor
2019-02-28 11:59 ` Greg KH
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Louis Taylor @ 2019-02-28 11:52 UTC (permalink / raw)
To: gregkh
Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat,
Louis Taylor
When compiling with -Wformat, clang warns:
./include/linux/usb/wusb.h:245:5: warning: format specifies type
'unsigned short' but the argument has type 'u8' (aka 'unsigned char')
[-Wformat]
ckhdid->data[0], ckhdid->data[1],
^~~~~~~~~~~~~~~
ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch
updates the format characters to the correct one for unsigned char types.
Link: https://github.com/ClangBuiltLinux/linux/issues/378
Signed-off-by: Louis Taylor <louis@kragniz.eu>
---
include/linux/usb/wusb.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h
index 9e4a3213f2c2..0a3cdf10972d 100644
--- a/include/linux/usb/wusb.h
+++ b/include/linux/usb/wusb.h
@@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size,
const struct wusb_ckhdid *ckhdid)
{
return scnprintf(pr_ckhdid, size,
- "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx "
- "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx",
+ "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu "
+ "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu",
ckhdid->data[0], ckhdid->data[1],
ckhdid->data[2], ckhdid->data[3],
ckhdid->data[4], ckhdid->data[5],
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] wusb: use correct format characters 2019-02-28 11:52 [PATCH] wusb: use correct format characters Louis Taylor @ 2019-02-28 11:59 ` Greg KH 2019-02-28 12:24 ` [PATCH v2] " Louis Taylor ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Greg KH @ 2019-02-28 11:59 UTC (permalink / raw) To: Louis Taylor Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat On Thu, Feb 28, 2019 at 11:52:56AM +0000, Louis Taylor wrote: > When compiling with -Wformat, clang warns: > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > [-Wformat] > ckhdid->data[0], ckhdid->data[1], > ^~~~~~~~~~~~~~~ > > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > updates the format characters to the correct one for unsigned char types. > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Louis Taylor <louis@kragniz.eu> > --- > include/linux/usb/wusb.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..0a3cdf10972d 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu " > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu", Why did you switch from hex to decimal in the output format? That feels very wrong to me. greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] wusb: use correct format characters 2019-02-28 11:52 [PATCH] wusb: use correct format characters Louis Taylor 2019-02-28 11:59 ` Greg KH @ 2019-02-28 12:24 ` Louis Taylor 2019-02-28 19:19 ` Nick Desaulniers 2019-02-28 21:53 ` Joe Perches 2019-03-05 16:21 ` [PATCH] wusb: use correct format characters Test 2019-03-05 16:28 ` Test 3 siblings, 2 replies; 14+ messages in thread From: Louis Taylor @ 2019-02-28 12:24 UTC (permalink / raw) To: gregkh Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat, Louis Taylor When compiling with -Wformat, clang warns: ./include/linux/usb/wusb.h:245:5: warning: format specifies type 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') [-Wformat] ckhdid->data[0], ckhdid->data[1], ^~~~~~~~~~~~~~~ ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch updates the format characters to the correct one for unsigned char types. Link: https://github.com/ClangBuiltLinux/linux/issues/378 Signed-off-by: Louis Taylor <louis@kragniz.eu> --- include/linux/usb/wusb.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h index 9e4a3213f2c2..625366d3499e 100644 --- a/include/linux/usb/wusb.h +++ b/include/linux/usb/wusb.h @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, const struct wusb_ckhdid *ckhdid) { return scnprintf(pr_ckhdid, size, - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx " + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx", ckhdid->data[0], ckhdid->data[1], ckhdid->data[2], ckhdid->data[3], ckhdid->data[4], ckhdid->data[5], -- 2.20.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] wusb: use correct format characters 2019-02-28 12:24 ` [PATCH v2] " Louis Taylor @ 2019-02-28 19:19 ` Nick Desaulniers 2019-02-28 21:53 ` Joe Perches 1 sibling, 0 replies; 14+ messages in thread From: Nick Desaulniers @ 2019-02-28 19:19 UTC (permalink / raw) To: Louis Taylor, Greg KH Cc: linux-usb, LKML, clang-built-linux, Jon Flatley, Stephen Hines, inaky.perez-gonzalez, david.vrabel, Miguel Ojeda On Thu, Feb 28, 2019 at 4:28 AM Louis Taylor <louis@kragniz.eu> wrote: > > When compiling with -Wformat, clang warns: > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') We should probably update Documentation/core-api/printk-formats.rst to list [u|s][8|16] printk formats so that I don't have to go read lib/vsprintf.c#format_decode(). (TODO in a separate patch) > [-Wformat] > ckhdid->data[0], ckhdid->data[1], > ^~~~~~~~~~~~~~~ > > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > updates the format characters to the correct one for unsigned char types. > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Louis Taylor <louis@kragniz.eu> > --- > include/linux/usb/wusb.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..625366d3499e 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx " > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx", Looks like lib/vsprintf.c#format_decode() accepts either %hh or %H for lone unsigned bytes, IIUC. Thanks for the patch and for following up on the feedback. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> I wonder if __attribute__((packed)) is needed on the definition of `struct wusb_ckhdid`? hmm... via Stephen: https://godbolt.org/z/vs5JNo Looks like alignof(struct wusb_ckhdid) is 1. And struct wusb_ckhdid was introduced as is in commit c7f736484f8e ("wusb: add the Wireless USB include files.") so it's not like it ever had additional members that would disturb the alignment. `__aligned(__alignof__(u8))` might be clearer than `__attribute__((packed))`, but even then, I don't think anything is necessary since the alignof should always be 1? > ckhdid->data[0], ckhdid->data[1], > ckhdid->data[2], ckhdid->data[3], > ckhdid->data[4], ckhdid->data[5], > -- > 2.20.1 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] wusb: use correct format characters 2019-02-28 12:24 ` [PATCH v2] " Louis Taylor 2019-02-28 19:19 ` Nick Desaulniers @ 2019-02-28 21:53 ` Joe Perches 2019-02-28 22:23 ` Jon Flatley 1 sibling, 1 reply; 14+ messages in thread From: Joe Perches @ 2019-02-28 21:53 UTC (permalink / raw) To: Louis Taylor, gregkh Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat On Thu, 2019-02-28 at 12:24 +0000, Louis Taylor wrote: > When compiling with -Wformat, clang warns: > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > [-Wformat] > ckhdid->data[0], ckhdid->data[1], > ^~~~~~~~~~~~~~~ I think the message is somewhat misguided as all the vararg arguments have implicit integer promotions. > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > updates the format characters to the correct one for unsigned char types. [] > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h [] > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx " > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx", > ckhdid->data[0], ckhdid->data[1], > ckhdid->data[2], ckhdid->data[3], > ckhdid->data[4], ckhdid->data[5], Better to use the vsprintf %ph extension insead. --- include/linux/usb/wusb.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h index 9e4a3213f2c2..8c39ddf62951 100644 --- a/include/linux/usb/wusb.h +++ b/include/linux/usb/wusb.h @@ -239,17 +239,7 @@ enum { static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, const struct wusb_ckhdid *ckhdid) { - return scnprintf(pr_ckhdid, size, - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", - ckhdid->data[0], ckhdid->data[1], - ckhdid->data[2], ckhdid->data[3], - ckhdid->data[4], ckhdid->data[5], - ckhdid->data[6], ckhdid->data[7], - ckhdid->data[8], ckhdid->data[9], - ckhdid->data[10], ckhdid->data[11], - ckhdid->data[12], ckhdid->data[13], - ckhdid->data[14], ckhdid->data[15]); + return scnprintf(pr_ckhdid, size, "%16ph", ckhdid->data); } /* ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] wusb: use correct format characters 2019-02-28 21:53 ` Joe Perches @ 2019-02-28 22:23 ` Jon Flatley 2019-02-28 23:05 ` Joe Perches 0 siblings, 1 reply; 14+ messages in thread From: Jon Flatley @ 2019-02-28 22:23 UTC (permalink / raw) To: Joe Perches Cc: Louis Taylor, gregkh, linux-usb, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers, Jon Flatley Thanks for the patch and comments. On Thu, Feb 28, 2019 at 1:53 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-02-28 at 12:24 +0000, Louis Taylor wrote: > > When compiling with -Wformat, clang warns: > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > > [-Wformat] > > ckhdid->data[0], ckhdid->data[1], > > ^~~~~~~~~~~~~~~ > > I think the message is somewhat misguided as all the > vararg arguments have implicit integer promotions. That's a fair point, but Clang checks the arguments against their format specifier before they're promoted when using -Wformat. When considering integer promotions it's difficult to say if this is "wrong", but since 'unsigned char' corresponds to the "hh" length specifier I don't think this is misguided. Otherwise, why use the "h" length specifier at all? > > > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > > updates the format characters to the correct one for unsigned char types. > [] > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > [] > > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > > const struct wusb_ckhdid *ckhdid) > > { > > return scnprintf(pr_ckhdid, size, > > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx " > > + "%02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx %02hhx", > > ckhdid->data[0], ckhdid->data[1], > > ckhdid->data[2], ckhdid->data[3], > > ckhdid->data[4], ckhdid->data[5], > > Better to use the vsprintf %ph extension insead. Agreed, so I guess my previous comment is irrelevant in this scenario. > --- > include/linux/usb/wusb.h | 12 +----------- > 1 file changed, 1 insertion(+), 11 deletions(-) > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..8c39ddf62951 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -239,17 +239,7 @@ enum { > static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > - return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > - ckhdid->data[0], ckhdid->data[1], > - ckhdid->data[2], ckhdid->data[3], > - ckhdid->data[4], ckhdid->data[5], > - ckhdid->data[6], ckhdid->data[7], > - ckhdid->data[8], ckhdid->data[9], > - ckhdid->data[10], ckhdid->data[11], > - ckhdid->data[12], ckhdid->data[13], > - ckhdid->data[14], ckhdid->data[15]); > + return scnprintf(pr_ckhdid, size, "%16ph", ckhdid->data); > } > > /* > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] wusb: use correct format characters 2019-02-28 22:23 ` Jon Flatley @ 2019-02-28 23:05 ` Joe Perches 2019-03-01 0:03 ` Jon Flatley 0 siblings, 1 reply; 14+ messages in thread From: Joe Perches @ 2019-02-28 23:05 UTC (permalink / raw) To: Jon Flatley Cc: Louis Taylor, gregkh, linux-usb, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers On Thu, 2019-02-28 at 14:23 -0800, Jon Flatley wrote: > Thanks for the patch and comments. > > On Thu, Feb 28, 2019 at 1:53 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-02-28 at 12:24 +0000, Louis Taylor wrote: > > > When compiling with -Wformat, clang warns: > > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > > > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > > > [-Wformat] > > > ckhdid->data[0], ckhdid->data[1], > > > ^~~~~~~~~~~~~~~ > > > > I think the message is somewhat misguided as all the > > vararg arguments have implicit integer promotions. > > That's a fair point, but Clang checks the arguments against their > format specifier before they're promoted when using -Wformat. Perhaps clang could be a bit more verbose if checking signed types emitted as unsigned or unsigned types emitted as signed instead. > When > considering integer promotions it's difficult to say if this is > "wrong", I didn't write "wrong", I wrote misguided. > but since 'unsigned char' corresponds to the "hh" length > specifier I don't think this is misguided. Otherwise, why use the "h" > length specifier at all? e.g.: signed char as %x needs %hhx cheers, Joe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] wusb: use correct format characters 2019-02-28 23:05 ` Joe Perches @ 2019-03-01 0:03 ` Jon Flatley 2019-03-01 4:38 ` [PATCH] wusb: Remove unnecessary static function ckhdid_printf Joe Perches 0 siblings, 1 reply; 14+ messages in thread From: Jon Flatley @ 2019-03-01 0:03 UTC (permalink / raw) To: Joe Perches Cc: Jon Flatley, Louis Taylor, gregkh, linux-usb, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers On Thu, Feb 28, 2019 at 3:05 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-02-28 at 14:23 -0800, Jon Flatley wrote: > > Thanks for the patch and comments. > > > > On Thu, Feb 28, 2019 at 1:53 PM Joe Perches <joe@perches.com> wrote: > > > On Thu, 2019-02-28 at 12:24 +0000, Louis Taylor wrote: > > > > When compiling with -Wformat, clang warns: > > > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > > > > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > > > > [-Wformat] > > > > ckhdid->data[0], ckhdid->data[1], > > > > ^~~~~~~~~~~~~~~ > > > > > > I think the message is somewhat misguided as all the > > > vararg arguments have implicit integer promotions. > > > > That's a fair point, but Clang checks the arguments against their > > format specifier before they're promoted when using -Wformat. > > Perhaps clang could be a bit more verbose if > checking signed types emitted as unsigned or > unsigned types emitted as signed instead. It is a little strange that clang warns when the length specifier doesn't match but not when an unsigned specifier is used for a signed value and vice versa. > > > When > > considering integer promotions it's difficult to say if this is > > "wrong", > > I didn't write "wrong", I wrote misguided. Apologies for my poor wording. I meant "wrong" in the sense that it's unclear if an improper length specifier is deserving of a warning. After all GCC doesn't warn for incorrect length specifiers, and as you pointed out Clang doesn't pay attention to if the specifier expects a signed or unsigned value. Cheers, Jon ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] wusb: Remove unnecessary static function ckhdid_printf 2019-03-01 0:03 ` Jon Flatley @ 2019-03-01 4:38 ` Joe Perches 2019-03-01 11:15 ` Louis Taylor 2019-03-01 18:18 ` Nick Desaulniers 0 siblings, 2 replies; 14+ messages in thread From: Joe Perches @ 2019-03-01 4:38 UTC (permalink / raw) To: Jon Flatley Cc: Louis Taylor, gregkh, linux-usb, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers This static inline is unnecessary and can be removed by using the vsprintf %ph extension. This reduces overall object size by more than 2K. Signed-off-by: Joe Perches <joe@perches.com> --- drivers/usb/wusbcore/cbaf.c | 15 ++++----------- drivers/usb/wusbcore/dev-sysfs.c | 5 ++--- drivers/usb/wusbcore/devconnect.c | 2 +- drivers/usb/wusbcore/wusbhc.c | 6 +----- include/linux/usb/wusb.h | 16 ---------------- 5 files changed, 8 insertions(+), 36 deletions(-) diff --git a/drivers/usb/wusbcore/cbaf.c b/drivers/usb/wusbcore/cbaf.c index 222228c5c1e1..af77064c7456 100644 --- a/drivers/usb/wusbcore/cbaf.c +++ b/drivers/usb/wusbcore/cbaf.c @@ -302,10 +302,8 @@ static ssize_t cbaf_wusb_chid_show(struct device *dev, { struct usb_interface *iface = to_usb_interface(dev); struct cbaf *cbaf = usb_get_intfdata(iface); - char pr_chid[WUSB_CKHDID_STRSIZE]; - ckhdid_printf(pr_chid, sizeof(pr_chid), &cbaf->chid); - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_chid); + return sprintf(buf, "%16ph\n", cbaf->chid.data); } static ssize_t cbaf_wusb_chid_store(struct device *dev, @@ -415,10 +413,8 @@ static ssize_t cbaf_wusb_cdid_show(struct device *dev, { struct usb_interface *iface = to_usb_interface(dev); struct cbaf *cbaf = usb_get_intfdata(iface); - char pr_cdid[WUSB_CKHDID_STRSIZE]; - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &cbaf->cdid); - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_cdid); + return sprintf(buf, "%16ph\n", cbaf->cdid.data); } static ssize_t cbaf_wusb_cdid_store(struct device *dev, @@ -503,7 +499,6 @@ static int cbaf_cc_upload(struct cbaf *cbaf) int result; struct device *dev = &cbaf->usb_iface->dev; struct wusb_cbaf_cc_data *ccd; - char pr_cdid[WUSB_CKHDID_STRSIZE]; ccd = cbaf->buffer; *ccd = cbaf_cc_data_defaults; @@ -513,10 +508,8 @@ static int cbaf_cc_upload(struct cbaf *cbaf) ccd->BandGroups = cpu_to_le16(cbaf->host_band_groups); dev_dbg(dev, "Trying to upload CC:\n"); - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CHID); - dev_dbg(dev, " CHID %s\n", pr_cdid); - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CDID); - dev_dbg(dev, " CDID %s\n", pr_cdid); + dev_dbg(dev, " CHID %16ph\n", ccd->CHID.data); + dev_dbg(dev, " CDID %16ph\n", ccd->CDID.data); dev_dbg(dev, " Bandgroups 0x%04x\n", cbaf->host_band_groups); result = usb_control_msg( diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c index 85a1acf3a729..67b0a4c412b2 100644 --- a/drivers/usb/wusbcore/dev-sysfs.c +++ b/drivers/usb/wusbcore/dev-sysfs.c @@ -50,10 +50,9 @@ static ssize_t wusb_cdid_show(struct device *dev, wusb_dev = wusb_dev_get_by_usb_dev(to_usb_device(dev)); if (wusb_dev == NULL) return -ENODEV; - result = ckhdid_printf(buf, PAGE_SIZE, &wusb_dev->cdid); - strcat(buf, "\n"); + result = sprintf(buf, "%16ph\n", wusb_dev->cdid.data); wusb_dev_put(wusb_dev); - return result + 1; + return result; } static DEVICE_ATTR_RO(wusb_cdid); diff --git a/drivers/usb/wusbcore/devconnect.c b/drivers/usb/wusbcore/devconnect.c index fcb06aef2675..a93837d57d53 100644 --- a/drivers/usb/wusbcore/devconnect.c +++ b/drivers/usb/wusbcore/devconnect.c @@ -532,7 +532,7 @@ static void wusbhc_handle_dn_connect(struct wusbhc *wusbhc, } dnc = container_of(dn_hdr, struct wusb_dn_connect, hdr); - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &dnc->CDID); + sprintf(pr_cdid, "%16ph", dnc->CDID.data); dev_info(dev, "DN CONNECT: device %s @ %x (%s) wants to %s\n", pr_cdid, wusb_dn_connect_prev_dev_addr(dnc), diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c index e5ba6140c1ba..d0b404d258e8 100644 --- a/drivers/usb/wusbcore/wusbhc.c +++ b/drivers/usb/wusbcore/wusbhc.c @@ -80,17 +80,13 @@ static ssize_t wusb_chid_show(struct device *dev, { struct wusbhc *wusbhc = usbhc_dev_to_wusbhc(dev); const struct wusb_ckhdid *chid; - ssize_t result = 0; if (wusbhc->wuie_host_info != NULL) chid = &wusbhc->wuie_host_info->CHID; else chid = &wusb_ckhdid_zero; - result += ckhdid_printf(buf, PAGE_SIZE, chid); - result += sprintf(buf + result, "\n"); - - return result; + return sprintf(buf, "%16ph\n", chid->data); } /* diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h index 9e4a3213f2c2..65adee629106 100644 --- a/include/linux/usb/wusb.h +++ b/include/linux/usb/wusb.h @@ -236,22 +236,6 @@ enum { WUSB_TRUST_TIMEOUT_MS = 4000, /* [WUSB] section 4.15.1 */ }; -static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, - const struct wusb_ckhdid *ckhdid) -{ - return scnprintf(pr_ckhdid, size, - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", - ckhdid->data[0], ckhdid->data[1], - ckhdid->data[2], ckhdid->data[3], - ckhdid->data[4], ckhdid->data[5], - ckhdid->data[6], ckhdid->data[7], - ckhdid->data[8], ckhdid->data[9], - ckhdid->data[10], ckhdid->data[11], - ckhdid->data[12], ckhdid->data[13], - ckhdid->data[14], ckhdid->data[15]); -} - /* * WUSB Crypto stuff (WUSB1.0[6]) */ ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] wusb: Remove unnecessary static function ckhdid_printf 2019-03-01 4:38 ` [PATCH] wusb: Remove unnecessary static function ckhdid_printf Joe Perches @ 2019-03-01 11:15 ` Louis Taylor 2019-03-01 18:18 ` Nick Desaulniers 1 sibling, 0 replies; 14+ messages in thread From: Louis Taylor @ 2019-03-01 11:15 UTC (permalink / raw) To: Joe Perches Cc: Jon Flatley, gregkh, linux-usb, Linux Kernel Mailing List, clang-built-linux, Nick Desaulniers On Thu, Feb 28, 2019 at 08:38:16PM -0800, Joe Perches wrote: > This static inline is unnecessary and can be removed > by using the vsprintf %ph extension. > > This reduces overall object size by more than 2K. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > drivers/usb/wusbcore/cbaf.c | 15 ++++----------- > drivers/usb/wusbcore/dev-sysfs.c | 5 ++--- > drivers/usb/wusbcore/devconnect.c | 2 +- > drivers/usb/wusbcore/wusbhc.c | 6 +----- > include/linux/usb/wusb.h | 16 ---------------- > 5 files changed, 8 insertions(+), 36 deletions(-) > > diff --git a/drivers/usb/wusbcore/cbaf.c b/drivers/usb/wusbcore/cbaf.c > index 222228c5c1e1..af77064c7456 100644 > --- a/drivers/usb/wusbcore/cbaf.c > +++ b/drivers/usb/wusbcore/cbaf.c > @@ -302,10 +302,8 @@ static ssize_t cbaf_wusb_chid_show(struct device *dev, > { > struct usb_interface *iface = to_usb_interface(dev); > struct cbaf *cbaf = usb_get_intfdata(iface); > - char pr_chid[WUSB_CKHDID_STRSIZE]; > > - ckhdid_printf(pr_chid, sizeof(pr_chid), &cbaf->chid); > - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_chid); > + return sprintf(buf, "%16ph\n", cbaf->chid.data); > } > > static ssize_t cbaf_wusb_chid_store(struct device *dev, > @@ -415,10 +413,8 @@ static ssize_t cbaf_wusb_cdid_show(struct device *dev, > { > struct usb_interface *iface = to_usb_interface(dev); > struct cbaf *cbaf = usb_get_intfdata(iface); > - char pr_cdid[WUSB_CKHDID_STRSIZE]; > > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &cbaf->cdid); > - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_cdid); > + return sprintf(buf, "%16ph\n", cbaf->cdid.data); > } > > static ssize_t cbaf_wusb_cdid_store(struct device *dev, > @@ -503,7 +499,6 @@ static int cbaf_cc_upload(struct cbaf *cbaf) > int result; > struct device *dev = &cbaf->usb_iface->dev; > struct wusb_cbaf_cc_data *ccd; > - char pr_cdid[WUSB_CKHDID_STRSIZE]; > > ccd = cbaf->buffer; > *ccd = cbaf_cc_data_defaults; > @@ -513,10 +508,8 @@ static int cbaf_cc_upload(struct cbaf *cbaf) > ccd->BandGroups = cpu_to_le16(cbaf->host_band_groups); > > dev_dbg(dev, "Trying to upload CC:\n"); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CHID); > - dev_dbg(dev, " CHID %s\n", pr_cdid); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CDID); > - dev_dbg(dev, " CDID %s\n", pr_cdid); > + dev_dbg(dev, " CHID %16ph\n", ccd->CHID.data); > + dev_dbg(dev, " CDID %16ph\n", ccd->CDID.data); > dev_dbg(dev, " Bandgroups 0x%04x\n", cbaf->host_band_groups); > > result = usb_control_msg( > diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c > index 85a1acf3a729..67b0a4c412b2 100644 > --- a/drivers/usb/wusbcore/dev-sysfs.c > +++ b/drivers/usb/wusbcore/dev-sysfs.c > @@ -50,10 +50,9 @@ static ssize_t wusb_cdid_show(struct device *dev, > wusb_dev = wusb_dev_get_by_usb_dev(to_usb_device(dev)); > if (wusb_dev == NULL) > return -ENODEV; > - result = ckhdid_printf(buf, PAGE_SIZE, &wusb_dev->cdid); > - strcat(buf, "\n"); > + result = sprintf(buf, "%16ph\n", wusb_dev->cdid.data); > wusb_dev_put(wusb_dev); > - return result + 1; > + return result; > } > static DEVICE_ATTR_RO(wusb_cdid); > > diff --git a/drivers/usb/wusbcore/devconnect.c b/drivers/usb/wusbcore/devconnect.c > index fcb06aef2675..a93837d57d53 100644 > --- a/drivers/usb/wusbcore/devconnect.c > +++ b/drivers/usb/wusbcore/devconnect.c > @@ -532,7 +532,7 @@ static void wusbhc_handle_dn_connect(struct wusbhc *wusbhc, > } > > dnc = container_of(dn_hdr, struct wusb_dn_connect, hdr); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &dnc->CDID); > + sprintf(pr_cdid, "%16ph", dnc->CDID.data); > dev_info(dev, "DN CONNECT: device %s @ %x (%s) wants to %s\n", > pr_cdid, > wusb_dn_connect_prev_dev_addr(dnc), > diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c > index e5ba6140c1ba..d0b404d258e8 100644 > --- a/drivers/usb/wusbcore/wusbhc.c > +++ b/drivers/usb/wusbcore/wusbhc.c > @@ -80,17 +80,13 @@ static ssize_t wusb_chid_show(struct device *dev, > { > struct wusbhc *wusbhc = usbhc_dev_to_wusbhc(dev); > const struct wusb_ckhdid *chid; > - ssize_t result = 0; > > if (wusbhc->wuie_host_info != NULL) > chid = &wusbhc->wuie_host_info->CHID; > else > chid = &wusb_ckhdid_zero; > > - result += ckhdid_printf(buf, PAGE_SIZE, chid); > - result += sprintf(buf + result, "\n"); > - > - return result; > + return sprintf(buf, "%16ph\n", chid->data); > } > > /* > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..65adee629106 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -236,22 +236,6 @@ enum { > WUSB_TRUST_TIMEOUT_MS = 4000, /* [WUSB] section 4.15.1 */ > }; > > -static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > - const struct wusb_ckhdid *ckhdid) > -{ > - return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > - ckhdid->data[0], ckhdid->data[1], > - ckhdid->data[2], ckhdid->data[3], > - ckhdid->data[4], ckhdid->data[5], > - ckhdid->data[6], ckhdid->data[7], > - ckhdid->data[8], ckhdid->data[9], > - ckhdid->data[10], ckhdid->data[11], > - ckhdid->data[12], ckhdid->data[13], > - ckhdid->data[14], ckhdid->data[15]); > -} This removes the original clang warning and seems like a nice cleanup. Reviewed-by: Louis Taylor <louis@kragniz.eu> Cheers, Louis ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] wusb: Remove unnecessary static function ckhdid_printf 2019-03-01 4:38 ` [PATCH] wusb: Remove unnecessary static function ckhdid_printf Joe Perches 2019-03-01 11:15 ` Louis Taylor @ 2019-03-01 18:18 ` Nick Desaulniers 2019-03-01 19:54 ` Greg KH 1 sibling, 1 reply; 14+ messages in thread From: Nick Desaulniers @ 2019-03-01 18:18 UTC (permalink / raw) To: Joe Perches Cc: Jon Flatley, Louis Taylor, Greg KH, linux-usb, Linux Kernel Mailing List, clang-built-linux On Thu, Feb 28, 2019 at 8:38 PM Joe Perches <joe@perches.com> wrote: > > This static inline is unnecessary and can be removed > by using the vsprintf %ph extension. > > This reduces overall object size by more than 2K. > > Signed-off-by: Joe Perches <joe@perches.com> Deserves a reported by tag from (https://lkml.org/lkml/2019/2/28/1138) Reported-by: Louis Taylor <louis@kragniz.eu> > --- > drivers/usb/wusbcore/cbaf.c | 15 ++++----------- > drivers/usb/wusbcore/dev-sysfs.c | 5 ++--- > drivers/usb/wusbcore/devconnect.c | 2 +- > drivers/usb/wusbcore/wusbhc.c | 6 +----- > include/linux/usb/wusb.h | 16 ---------------- > 5 files changed, 8 insertions(+), 36 deletions(-) > > diff --git a/drivers/usb/wusbcore/cbaf.c b/drivers/usb/wusbcore/cbaf.c > index 222228c5c1e1..af77064c7456 100644 > --- a/drivers/usb/wusbcore/cbaf.c > +++ b/drivers/usb/wusbcore/cbaf.c > @@ -302,10 +302,8 @@ static ssize_t cbaf_wusb_chid_show(struct device *dev, > { > struct usb_interface *iface = to_usb_interface(dev); > struct cbaf *cbaf = usb_get_intfdata(iface); > - char pr_chid[WUSB_CKHDID_STRSIZE]; > > - ckhdid_printf(pr_chid, sizeof(pr_chid), &cbaf->chid); > - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_chid); > + return sprintf(buf, "%16ph\n", cbaf->chid.data); > } > > static ssize_t cbaf_wusb_chid_store(struct device *dev, > @@ -415,10 +413,8 @@ static ssize_t cbaf_wusb_cdid_show(struct device *dev, > { > struct usb_interface *iface = to_usb_interface(dev); > struct cbaf *cbaf = usb_get_intfdata(iface); > - char pr_cdid[WUSB_CKHDID_STRSIZE]; > > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &cbaf->cdid); > - return scnprintf(buf, PAGE_SIZE, "%s\n", pr_cdid); > + return sprintf(buf, "%16ph\n", cbaf->cdid.data); > } Great simplification overall. If you wanted to push further, cbaf_wusb_c{h|d}id_show() are basically (almost) the same function now. They probably could share more code. Everything else looks good, thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > static ssize_t cbaf_wusb_cdid_store(struct device *dev, > @@ -503,7 +499,6 @@ static int cbaf_cc_upload(struct cbaf *cbaf) > int result; > struct device *dev = &cbaf->usb_iface->dev; > struct wusb_cbaf_cc_data *ccd; > - char pr_cdid[WUSB_CKHDID_STRSIZE]; > > ccd = cbaf->buffer; > *ccd = cbaf_cc_data_defaults; > @@ -513,10 +508,8 @@ static int cbaf_cc_upload(struct cbaf *cbaf) > ccd->BandGroups = cpu_to_le16(cbaf->host_band_groups); > > dev_dbg(dev, "Trying to upload CC:\n"); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CHID); > - dev_dbg(dev, " CHID %s\n", pr_cdid); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &ccd->CDID); > - dev_dbg(dev, " CDID %s\n", pr_cdid); > + dev_dbg(dev, " CHID %16ph\n", ccd->CHID.data); > + dev_dbg(dev, " CDID %16ph\n", ccd->CDID.data); > dev_dbg(dev, " Bandgroups 0x%04x\n", cbaf->host_band_groups); > > result = usb_control_msg( > diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c > index 85a1acf3a729..67b0a4c412b2 100644 > --- a/drivers/usb/wusbcore/dev-sysfs.c > +++ b/drivers/usb/wusbcore/dev-sysfs.c > @@ -50,10 +50,9 @@ static ssize_t wusb_cdid_show(struct device *dev, > wusb_dev = wusb_dev_get_by_usb_dev(to_usb_device(dev)); > if (wusb_dev == NULL) > return -ENODEV; > - result = ckhdid_printf(buf, PAGE_SIZE, &wusb_dev->cdid); > - strcat(buf, "\n"); > + result = sprintf(buf, "%16ph\n", wusb_dev->cdid.data); > wusb_dev_put(wusb_dev); > - return result + 1; > + return result; > } > static DEVICE_ATTR_RO(wusb_cdid); > > diff --git a/drivers/usb/wusbcore/devconnect.c b/drivers/usb/wusbcore/devconnect.c > index fcb06aef2675..a93837d57d53 100644 > --- a/drivers/usb/wusbcore/devconnect.c > +++ b/drivers/usb/wusbcore/devconnect.c > @@ -532,7 +532,7 @@ static void wusbhc_handle_dn_connect(struct wusbhc *wusbhc, > } > > dnc = container_of(dn_hdr, struct wusb_dn_connect, hdr); > - ckhdid_printf(pr_cdid, sizeof(pr_cdid), &dnc->CDID); > + sprintf(pr_cdid, "%16ph", dnc->CDID.data); > dev_info(dev, "DN CONNECT: device %s @ %x (%s) wants to %s\n", > pr_cdid, > wusb_dn_connect_prev_dev_addr(dnc), > diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c > index e5ba6140c1ba..d0b404d258e8 100644 > --- a/drivers/usb/wusbcore/wusbhc.c > +++ b/drivers/usb/wusbcore/wusbhc.c > @@ -80,17 +80,13 @@ static ssize_t wusb_chid_show(struct device *dev, > { > struct wusbhc *wusbhc = usbhc_dev_to_wusbhc(dev); > const struct wusb_ckhdid *chid; > - ssize_t result = 0; > > if (wusbhc->wuie_host_info != NULL) > chid = &wusbhc->wuie_host_info->CHID; > else > chid = &wusb_ckhdid_zero; > > - result += ckhdid_printf(buf, PAGE_SIZE, chid); > - result += sprintf(buf + result, "\n"); > - > - return result; > + return sprintf(buf, "%16ph\n", chid->data); > } > > /* > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..65adee629106 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -236,22 +236,6 @@ enum { > WUSB_TRUST_TIMEOUT_MS = 4000, /* [WUSB] section 4.15.1 */ > }; > > -static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > - const struct wusb_ckhdid *ckhdid) > -{ > - return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > - ckhdid->data[0], ckhdid->data[1], > - ckhdid->data[2], ckhdid->data[3], > - ckhdid->data[4], ckhdid->data[5], > - ckhdid->data[6], ckhdid->data[7], > - ckhdid->data[8], ckhdid->data[9], > - ckhdid->data[10], ckhdid->data[11], > - ckhdid->data[12], ckhdid->data[13], > - ckhdid->data[14], ckhdid->data[15]); > -} > - > /* > * WUSB Crypto stuff (WUSB1.0[6]) > */ > > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] wusb: Remove unnecessary static function ckhdid_printf 2019-03-01 18:18 ` Nick Desaulniers @ 2019-03-01 19:54 ` Greg KH 0 siblings, 0 replies; 14+ messages in thread From: Greg KH @ 2019-03-01 19:54 UTC (permalink / raw) To: Nick Desaulniers Cc: Joe Perches, Jon Flatley, Louis Taylor, linux-usb, Linux Kernel Mailing List, clang-built-linux On Fri, Mar 01, 2019 at 10:18:32AM -0800, Nick Desaulniers wrote: > On Thu, Feb 28, 2019 at 8:38 PM Joe Perches <joe@perches.com> wrote: > > > > This static inline is unnecessary and can be removed > > by using the vsprintf %ph extension. > > > > This reduces overall object size by more than 2K. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > Deserves a reported by tag from (https://lkml.org/lkml/2019/2/28/1138) > Reported-by: Louis Taylor <louis@kragniz.eu> Good idea, will go add that... ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] wusb: use correct format characters 2019-02-28 11:52 [PATCH] wusb: use correct format characters Louis Taylor 2019-02-28 11:59 ` Greg KH 2019-02-28 12:24 ` [PATCH v2] " Louis Taylor @ 2019-03-05 16:21 ` Test 2019-03-05 16:28 ` Test 3 siblings, 0 replies; 14+ messages in thread From: Test @ 2019-03-05 16:21 UTC (permalink / raw) Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat Test Am 28.02.19 um 12:52 schrieb Louis Taylor: > When compiling with -Wformat, clang warns: > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > [-Wformat] > ckhdid->data[0], ckhdid->data[1], > ^~~~~~~~~~~~~~~ > > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > updates the format characters to the correct one for unsigned char types. > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Louis Taylor <louis@kragniz.eu> > --- > include/linux/usb/wusb.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..0a3cdf10972d 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu " > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu", > ckhdid->data[0], ckhdid->data[1], > ckhdid->data[2], ckhdid->data[3], > ckhdid->data[4], ckhdid->data[5], ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] wusb: use correct format characters 2019-02-28 11:52 [PATCH] wusb: use correct format characters Louis Taylor ` (2 preceding siblings ...) 2019-03-05 16:21 ` [PATCH] wusb: use correct format characters Test @ 2019-03-05 16:28 ` Test 3 siblings, 0 replies; 14+ messages in thread From: Test @ 2019-03-05 16:28 UTC (permalink / raw) To: Louis Taylor, gregkh Cc: linux-usb, linux-kernel, clang-built-linux, ndesaulniers, jflat Hallo Am 28.02.19 um 12:52 schrieb Louis Taylor: > When compiling with -Wformat, clang warns: > > ./include/linux/usb/wusb.h:245:5: warning: format specifies type > 'unsigned short' but the argument has type 'u8' (aka 'unsigned char') > [-Wformat] > ckhdid->data[0], ckhdid->data[1], > ^~~~~~~~~~~~~~~ > > ckhdid->data is unconditionally defined as `u8 data[16]`, so this patch > updates the format characters to the correct one for unsigned char types. > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Louis Taylor <louis@kragniz.eu> > --- > include/linux/usb/wusb.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h > index 9e4a3213f2c2..0a3cdf10972d 100644 > --- a/include/linux/usb/wusb.h > +++ b/include/linux/usb/wusb.h > @@ -240,8 +240,8 @@ static inline size_t ckhdid_printf(char *pr_ckhdid, size_t size, > const struct wusb_ckhdid *ckhdid) > { > return scnprintf(pr_ckhdid, size, > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx " > - "%02hx %02hx %02hx %02hx %02hx %02hx %02hx %02hx", > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu " > + "%02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu %02hhu", > ckhdid->data[0], ckhdid->data[1], > ckhdid->data[2], ckhdid->data[3], > ckhdid->data[4], ckhdid->data[5], ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-03-05 16:29 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-28 11:52 [PATCH] wusb: use correct format characters Louis Taylor 2019-02-28 11:59 ` Greg KH 2019-02-28 12:24 ` [PATCH v2] " Louis Taylor 2019-02-28 19:19 ` Nick Desaulniers 2019-02-28 21:53 ` Joe Perches 2019-02-28 22:23 ` Jon Flatley 2019-02-28 23:05 ` Joe Perches 2019-03-01 0:03 ` Jon Flatley 2019-03-01 4:38 ` [PATCH] wusb: Remove unnecessary static function ckhdid_printf Joe Perches 2019-03-01 11:15 ` Louis Taylor 2019-03-01 18:18 ` Nick Desaulniers 2019-03-01 19:54 ` Greg KH 2019-03-05 16:21 ` [PATCH] wusb: use correct format characters Test 2019-03-05 16:28 ` Test
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).