Re: mt7621 static check warning

[Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>]

Hi Bjorn,

Thanks for the report.

On Wed, Jan 10, 2024 at 12:51 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> Hi Sergio,
>
> FYI:
>
>   $ make W=1 drivers/pci/
>     CC      drivers/pci/controller/pcie-mt7621.o
>   drivers/pci/controller/pcie-mt7621.c: In function ‘mt7621_pcie_probe’:
>   drivers/pci/controller/pcie-mt7621.c:228:49: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
>     228 |         snprintf(name, sizeof(name), "pcie-phy%d", slot);
>         |                                                 ^
>   drivers/pci/controller/pcie-mt7621.c:228:9: note: ‘snprintf’ output between 10 and 11 bytes into a destination of size 10
>     228 |         snprintf(name, sizeof(name), "pcie-phy%d", slot);
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>

Would you be happy if I just increment the buffer as follows?

diff --git a/drivers/pci/controller/pcie-mt7621.c
b/drivers/pci/controller/pcie-mt7621.c
index 79e225edb42a..d97b956e6e57 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -202,7 +202,7 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
        struct mt7621_pcie_port *port;
        struct device *dev = pcie->dev;
        struct platform_device *pdev = to_platform_device(dev);
-       char name[10];
+       char name[11];
        int err;

        port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);

Or should I use scnprintf instead? Since the statement is not using
function return value at all snprintf looks more correct and simpler
at a first glance.

diff --git a/drivers/pci/controller/pcie-mt7621.c
b/drivers/pci/controller/pcie-mt7621.c
index 79e225edb42a..0eae1b5b079e 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -225,7 +225,7 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
                return PTR_ERR(port->pcie_rst);
        }

-       snprintf(name, sizeof(name), "pcie-phy%d", slot);
+       scnprintf(name, sizeof(name), "pcie-phy%d", slot);
        port->phy = devm_of_phy_get(dev, node, name);
        if (IS_ERR(port->phy)) {
                dev_err(dev, "failed to get pcie-phy%d\n", slot);


Both of them silence the warning, so let me know your preference here.

> I know we'll never actually hit this, but it'd be nice to clean this
> up, and I don't think it would really cost us anything.  I think it's
> currently the only "W=1" warning in drivers/pci/.

I am also getting this:

drivers/pci/controller/dwc/pci-dra7xx.c: In function ‘dra7xx_pcie_probe’:
drivers/pci/controller/dwc/pci-dra7xx.c:754:41: error: ‘%d’ directive
output may be truncated writing between 1 and 10 bytes into a region
of size 2 [-Werror=format-truncation=]
  754 |   snprintf(name, sizeof(name), "pcie-phy%d", i);
      |                                         ^~
drivers/pci/controller/dwc/pci-dra7xx.c:754:32: note: directive
argument in the range [0, 2147483646]
  754 |   snprintf(name, sizeof(name), "pcie-phy%d", i);
      |                                ^~~~~~~~~~~~
drivers/pci/controller/dwc/pci-dra7xx.c:754:3: note: ‘snprintf’ output
between 10 and 19 bytes into a destination of size 10
  754 |   snprintf(name, sizeof(name), "pcie-phy%d", i);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Best regards,
    Sergio Paracuellos

>
> Bjorn