| Message ID | 20220708232653.556488-1-justinstitt@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2afe46474ba3fd3ae6e016bab57efd73f4b96175 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | amd-xgbe: fix clang -Wformat warnings | expand |
On Fri, Jul 8, 2022 at 4:27 PM Justin Stitt <justinstitt@google.com> wrote: > > When building with Clang we encounter the following warning: > | drivers/net/ethernet/amd/xgbe/xgbe-dcb.c:234:42: error: format specifies > | type 'unsigned char' but the argument has type '__u16' (aka 'unsigned > | short') [-Werror,-Wformat] pfc->pfc_cap, pfc->pfc_en, pfc->mbc, > | pfc->delay); > > pfc->pfc_cap , pfc->pfc_cn, pfc->mbc are all of type `u8` while pfc->delay is > of type `u16`. The correct format specifiers `%hh[u|x]` were used for > the first three but not for pfc->delay, which is causing the warning > above. > > Variadic functions (printf-like) undergo default argument promotion. > Documentation/core-api/printk-formats.rst specifically recommends using > the promoted-to-type's format flag. In this case `%d` (or `%x` to > maintain hex representation) should be used since both u8's and u16's > are fully representable by an int. > > Moreover, C11 6.3.1.1 states: > (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf) `If an int > can represent all values of the original type ..., the value is > converted to an int; otherwise, it is converted to an unsigned int. > These are called the integer promotions.` > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > Signed-off-by: Justin Stitt <justinstitt@google.com> Thanks for the patch, this fixes an instance of -Wformat I observe for x86_64 allmodconfig. I thought you had already fixed this file up? https://lore.kernel.org/llvm/20220607191119.20686-1-jstitt007@gmail.com/ which was merged. I'm guessing that was from a defconfig, while this was from an allmodconfig? Seems like there's a bunch of config ifdef'ery around the definition of netif_dbg. Either way, thanks for the patches and Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > --- > For clarification, the first three parameters given to netif_dbg did NOT > cause a -Wformat warning. I changed them simply to follow what the > standard and documentation recommend. > > drivers/net/ethernet/amd/xgbe/xgbe-dcb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > index 895d35639129..c68ace804e37 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > @@ -230,7 +230,7 @@ static int xgbe_dcb_ieee_setpfc(struct net_device *netdev, > struct xgbe_prv_data *pdata = netdev_priv(netdev); > > netif_dbg(pdata, drv, netdev, > - "cap=%hhu, en=%#hhx, mbc=%hhu, delay=%hhu\n", > + "cap=%d, en=%#x, mbc=%d, delay=%d\n", > pfc->pfc_cap, pfc->pfc_en, pfc->mbc, pfc->delay); > > /* Check PFC for supported number of traffic classes */ > -- > 2.37.0.rc0.161.g10f37bed90-goog >
On Fri, Jul 8, 2022 at 4:48 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Fri, Jul 8, 2022 at 4:27 PM Justin Stitt <justinstitt@google.com> wrote: > > > > When building with Clang we encounter the following warning: > > | drivers/net/ethernet/amd/xgbe/xgbe-dcb.c:234:42: error: format specifies > > | type 'unsigned char' but the argument has type '__u16' (aka 'unsigned > > | short') [-Werror,-Wformat] pfc->pfc_cap, pfc->pfc_en, pfc->mbc, > > | pfc->delay); > > > > pfc->pfc_cap , pfc->pfc_cn, pfc->mbc are all of type `u8` while pfc->delay is > > of type `u16`. The correct format specifiers `%hh[u|x]` were used for > > the first three but not for pfc->delay, which is causing the warning > > above. > > > > Variadic functions (printf-like) undergo default argument promotion. > > Documentation/core-api/printk-formats.rst specifically recommends using > > the promoted-to-type's format flag. In this case `%d` (or `%x` to > > maintain hex representation) should be used since both u8's and u16's > > are fully representable by an int. > > > > Moreover, C11 6.3.1.1 states: > > (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf) `If an int > > can represent all values of the original type ..., the value is > > converted to an int; otherwise, it is converted to an unsigned int. > > These are called the integer promotions.` > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/378 > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > Thanks for the patch, this fixes an instance of -Wformat I observe for > x86_64 allmodconfig. I thought you had already fixed this file up? > https://lore.kernel.org/llvm/20220607191119.20686-1-jstitt007@gmail.com/ > which was merged. I'm guessing that was from a defconfig, while this > was from an allmodconfig? > Seems like there's a bunch of config ifdef'ery around the definition > of netif_dbg. Definitely different across configs but ALSO files are ever-so-slightly different: amd/xgbe/xgbe-drv.c _v.s_ amd/xgbe/xgbe-dcb.c > > Either way, thanks for the patches and > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > For clarification, the first three parameters given to netif_dbg did NOT > > cause a -Wformat warning. I changed them simply to follow what the > > standard and documentation recommend. > > > > drivers/net/ethernet/amd/xgbe/xgbe-dcb.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > > index 895d35639129..c68ace804e37 100644 > > --- a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c > > @@ -230,7 +230,7 @@ static int xgbe_dcb_ieee_setpfc(struct net_device *netdev, > > struct xgbe_prv_data *pdata = netdev_priv(netdev); > > > > netif_dbg(pdata, drv, netdev, > > - "cap=%hhu, en=%#hhx, mbc=%hhu, delay=%hhu\n", > > + "cap=%d, en=%#x, mbc=%d, delay=%d\n", > > pfc->pfc_cap, pfc->pfc_en, pfc->mbc, pfc->delay); > > > > /* Check PFC for supported number of traffic classes */ > > -- > > 2.37.0.rc0.161.g10f37bed90-goog > > > > > -- > Thanks, > ~Nick Desaulniers
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 8 Jul 2022 16:26:53 -0700 you wrote: > When building with Clang we encounter the following warning: > | drivers/net/ethernet/amd/xgbe/xgbe-dcb.c:234:42: error: format specifies > | type 'unsigned char' but the argument has type '__u16' (aka 'unsigned > | short') [-Werror,-Wformat] pfc->pfc_cap, pfc->pfc_en, pfc->mbc, > | pfc->delay); > > pfc->pfc_cap , pfc->pfc_cn, pfc->mbc are all of type `u8` while pfc->delay is > of type `u16`. The correct format specifiers `%hh[u|x]` were used for > the first three but not for pfc->delay, which is causing the warning > above. > > [...] Here is the summary with links: - amd-xgbe: fix clang -Wformat warnings https://git.kernel.org/netdev/net-next/c/2afe46474ba3 You are awesome, thank you!
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c index 895d35639129..c68ace804e37 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dcb.c @@ -230,7 +230,7 @@ static int xgbe_dcb_ieee_setpfc(struct net_device *netdev, struct xgbe_prv_data *pdata = netdev_priv(netdev); netif_dbg(pdata, drv, netdev, - "cap=%hhu, en=%#hhx, mbc=%hhu, delay=%hhu\n", + "cap=%d, en=%#x, mbc=%d, delay=%d\n", pfc->pfc_cap, pfc->pfc_en, pfc->mbc, pfc->delay); /* Check PFC for supported number of traffic classes */
When building with Clang we encounter the following warning: | drivers/net/ethernet/amd/xgbe/xgbe-dcb.c:234:42: error: format specifies | type 'unsigned char' but the argument has type '__u16' (aka 'unsigned | short') [-Werror,-Wformat] pfc->pfc_cap, pfc->pfc_en, pfc->mbc, | pfc->delay); pfc->pfc_cap , pfc->pfc_cn, pfc->mbc are all of type `u8` while pfc->delay is of type `u16`. The correct format specifiers `%hh[u|x]` were used for the first three but not for pfc->delay, which is causing the warning above. Variadic functions (printf-like) undergo default argument promotion. Documentation/core-api/printk-formats.rst specifically recommends using the promoted-to-type's format flag. In this case `%d` (or `%x` to maintain hex representation) should be used since both u8's and u16's are fully representable by an int. Moreover, C11 6.3.1.1 states: (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf) `If an int can represent all values of the original type ..., the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions.` Link: https://github.com/ClangBuiltLinux/linux/issues/378 Signed-off-by: Justin Stitt <justinstitt@google.com> --- For clarification, the first three parameters given to netif_dbg did NOT cause a -Wformat warning. I changed them simply to follow what the standard and documentation recommend. drivers/net/ethernet/amd/xgbe/xgbe-dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)