Re: [PATCH v6 06/15] net/xsc: initialize xsc representors

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 20 Jan 2025 19:14:44 +0800
"WanRenyong" <wanry@yunsilicon.com> wrote:

> +static int
> +xsc_ethdev_init_representors(struct rte_eth_dev *eth_dev)
> +{
> +	struct xsc_ethdev_priv *priv = TO_XSC_ETHDEV_PRIV(eth_dev);
> +	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
> +	struct rte_device *dev;
> +	struct xsc_dev *xdev;
> +	struct xsc_repr_port *repr_port;
> +	char name[RTE_ETH_NAME_MAX_LEN];
> +	int i;
> +	int ret;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	dev = &priv->pci_dev->device;
> +	if (dev->devargs != NULL) {
> +		ret = rte_eth_devargs_parse(dev->devargs->args, &eth_da, 1);
> +		if (ret < 0) {
> +			PMD_DRV_LOG(ERR, "Failed to parse device arguments: %s",
> +				    dev->devargs->args);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	xdev = priv->xdev;
> +	ret = xsc_dev_repr_ports_probe(xdev, eth_da.nb_representor_ports, RTE_MAX_ETHPORTS);
> +	if (ret != 0) {
> +		PMD_DRV_LOG(ERR, "Failed to probe %d xsc device representors",
> +			    eth_da.nb_representor_ports);
> +		return ret;
> +	}
> +
> +	/* PF rep init */
> +	repr_port = &xdev->repr_ports[xdev->num_repr_ports - 1];
> +	ret = xsc_ethdev_init_one_representor(eth_dev, repr_port);
> +	if (ret != 0) {
> +		PMD_DRV_LOG(ERR, "Failed to init backing representor");
> +		return ret;
> +	}
> +
> +	/* VF rep init */
> +	for (i = 0; i < eth_da.nb_representor_ports; i++) {
> +		repr_port = &xdev->repr_ports[i];
> +		snprintf(name, sizeof(name), "%s_rep_%d",
> +			 xdev->name, repr_port->info.repr_id);


If I enable format-truncation warnings then there is this message.
The warning is suppressed by drivers/meson.build, but would like in the future
to not disable warnings there.

../drivers/net/xsc/xsc_ethdev.c: In function ‘xsc_ethdev_init’:
../drivers/net/xsc/xsc_ethdev.c:787:49: warning: ‘_rep_’ directive output may be truncated writing 5 bytes into a region of size between 1 and 64 [-Wformat-truncation=]
  787 |                 snprintf(name, sizeof(name), "%s_rep_%d",
      |                                                 ^~~~~
In function ‘xsc_ethdev_init_representors’,
    inlined from ‘xsc_ethdev_init’ at ../drivers/net/xsc/xsc_ethdev.c:834:8:
../drivers/net/xsc/xsc_ethdev.c:787:17: note: ‘snprintf’ output between 7 and 80 bytes into a destination of size 64
  787 |                 snprintf(name, sizeof(name), "%s_rep_%d",
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  788 |                          xdev->name, repr_port->info.repr_id);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is because local variable name is of size RTE_ETH_NAME_MAX_LEN which is 64
and xdev->name is defined as RTE_ETH_NAME_MAX_LEN as well.
The warning is correct but impossible to happen since xdev->name is only filled
in with PCI information. A simple way to get rid of it (and save some space)
would be to define name to be for PCI address only.

Would this work?

diff --git a/drivers/net/xsc/xsc_dev.h b/drivers/net/xsc/xsc_dev.h
index d661523e13..cc5fa8aad8 100644
--- a/drivers/net/xsc/xsc_dev.h
+++ b/drivers/net/xsc/xsc_dev.h
@@ -121,7 +121,7 @@ struct xsc_dev {
        int ifindex;
        int port_id; /* Probe dev */
        void *dev_priv;
-       char name[RTE_ETH_NAME_MAX_LEN];
+       char name[PCI_PRI_STR_SIZE];
        void *bar_addr;
        void *jumbo_buffer_pa;
        void *jumbo_buffer_va;