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

[Dan Carpenter <dan.carpenter@linaro.org>]

On Mon, Sep 04, 2023 at 12:58:55AM +0100, Andre Przywara wrote:
> On Sun,  3 Sep 2023 12:11:06 +0200
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> 
> > 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