Re: mt7621 static check warning

Sergio Paracuellos
2024-01-10T07:16:55Z

Hi Bjorn,

Thanks for the report.

On Wed, Jan 10, 2024 at 12:51 AM Bjorn Helgaas <helgaas@kernel.org> 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