Messages in this thread | | | Date | Tue, 12 Jul 2022 14:40:15 +0300 | From | Serge Semin <> | Subject | Re: [PATCH] ntb: idt: fix clang -Wformat warnings |
| |
Hi Justin
On Mon, Jul 11, 2022 at 04:01:48PM -0700, Justin Stitt wrote: > When building with Clang we encounter these warnings: > | drivers/ntb/hw/idt/ntb_hw_idt.c:2409:28: error: format specifies type > | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat] > | "\t%hhu-%hhu.\t", idx + cnt - 1); > - > | drivers/ntb/hw/idt/ntb_hw_idt.c:2438:29: error: format specifies type > | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat] > | "\t%hhu-%hhu.\t", idx + cnt - 1); > - > | drivers/ntb/hw/idt/ntb_hw_idt.c:2484:15: error: format specifies type > | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat], src); > > For the first two warnings the format specifier used is `%hhu` which > describes a u8. Both `idx` and `cnt` are u8 as well. However, the > expression as a whole is promoted to an int as you cannot get > smaller-than-int from addition. Therefore, to fix the warning, use the > promoted-to-type's format specifier -- in this case `%d`. > > example: > `` > uint8_t a = 4, b = 7; > int size = sizeof(a + b - 1); > printf("%d\n", size); > // output: 4 > ``` > > For the last warning, src is of type `int` while the format specifier > describes a u8. The fix here is just to use the proper specifier `%d`. > > See more: > (https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules) > "Integer types smaller than int are promoted when an operation is > performed on them. If all values of the original type can be represented > as an int, the value of the smaller type is converted to an int; > otherwise, it is converted to an unsigned int." > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Justin Stitt <justinstitt@google.com>
Thanks for fixing this. Definitely Acked-by: Serge Semin <fancer.lancer@gmail.com>
-Sergey
> --- > Note: This patch silences the -Wformat warnings for this file (which is > the goal) but in reality all instances of `%hh[dux]` should be converted > to `%[dux]` for this file and probably every file. That's a bit larger > scope than the goal of enabling -Wformat for Clang builds, though. > > drivers/ntb/hw/idt/ntb_hw_idt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c > index 733557231ed0..0ed6f809ff2e 100644 > --- a/drivers/ntb/hw/idt/ntb_hw_idt.c > +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c > @@ -2406,7 +2406,7 @@ static ssize_t idt_dbgfs_info_read(struct file *filp, char __user *ubuf, > "\t%hhu.\t", idx); > else > off += scnprintf(strbuf + off, size - off, > - "\t%hhu-%hhu.\t", idx, idx + cnt - 1); > + "\t%hhu-%d.\t", idx, idx + cnt - 1); > > off += scnprintf(strbuf + off, size - off, "%s BAR%hhu, ", > idt_get_mw_name(data), ndev->mws[idx].bar); > @@ -2435,7 +2435,7 @@ static ssize_t idt_dbgfs_info_read(struct file *filp, char __user *ubuf, > "\t%hhu.\t", idx); > else > off += scnprintf(strbuf + off, size - off, > - "\t%hhu-%hhu.\t", idx, idx + cnt - 1); > + "\t%hhu-%d.\t", idx, idx + cnt - 1); > > off += scnprintf(strbuf + off, size - off, > "%s BAR%hhu, ", idt_get_mw_name(data), > @@ -2480,7 +2480,7 @@ static ssize_t idt_dbgfs_info_read(struct file *filp, char __user *ubuf, > int src; > data = idt_ntb_msg_read(&ndev->ntb, &src, idx); > off += scnprintf(strbuf + off, size - off, > - "\t%hhu. 0x%08x from peer %hhu (Port %hhu)\n", > + "\t%hhu. 0x%08x from peer %d (Port %hhu)\n", > idx, data, src, ndev->peers[src].port); > } > off += scnprintf(strbuf + off, size - off, "\n"); > -- > 2.37.0.144.g8ac04bfd2-goog >
| |