Thread (9 messages) 9 messages, 5 authors, 2023-10-13

Re: [PATCH] phy: sun4i-usb: Fix a W=1 compilation failure

From: Dan Carpenter <hidden>
Date: 2023-09-05 17:09:07
Also in: kernel-janitors, linux-phy, linux-sunxi, lkml

On Mon, Sep 04, 2023 at 12:58:55AM +0100, Andre Przywara wrote:
On Sun,  3 Sep 2023 12:11:06 +0200
Christophe JAILLET [off-list ref] wrote:
quoted
With gcc 12.3.0, when this file is built, we get errors such as:

drivers/phy/allwinner/phy-sun4i-usb.c: In function ‘sun4i_usb_phy_probe’:
drivers/phy/allwinner/phy-sun4i-usb.c:790:52: error: ‘_vbus’ directive output may be truncated writing 5 bytes into a region of size between 2 and 12 [-Werror=format-truncation=]
  790 |                 snprintf(name, sizeof(name), "usb%d_vbus", i);
      |                                                    ^~~~~
drivers/phy/allwinner/phy-sun4i-usb.c:790:17: note: ‘snprintf’ output between 10 and 20 bytes into a destination of size 16
  790 |                 snprintf(name, sizeof(name), "usb%d_vbus", i);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Because of the possible value of 'i', this can't be an issue in real world
Would using "u8 i;" help? After all currently there are only 4 PHYs
max, and in general this isn't expected to be more than a "handful", so
8 bits should be plenty. An unsigned is better anyway.
Generally unsigned types are trickier and cause bugs.  I've blogged
about this before.  The title is a probably more negative than it should
have been.

https://staticthinking.wordpress.com/2022/06/01/unsigned-int-i-is-stupid/

My blog mentions u8 i.  I would say avoid that unless forced by an API.
It leaves a bit of a bitter taste, though, as we shouldn't do this kind
type tweaking, especially not to work around the compiler trying to be
clever, but then not seeing the whole picture (that "i" is bounded by
compile time constants not exceeding "4").
Yeah.  There is always the option of just ignoring the static checker
when it tells you to write bad code.

You don't have to even look at the *whole* picture to know that GCC is
wrong.  The BIT(i) would overflow if i is more than 31.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help