2023-06-15 08:23:38

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net-next v7 08/11] net: bcmasp: Add support for ethtool driver stats

On Wed, Jun 14, 2023 at 03:30:17PM -0700, Justin Chen wrote:

...

> +static void bcmasp_update_mib_counters(struct bcmasp_intf *intf)
> +{
> + int i;
> +
> + for (i = 0; i < BCMASP_STATS_LEN; i++) {
> + const struct bcmasp_stats *s;
> + u32 offset, val;
> + char *p;
> +
> + s = &bcmasp_gstrings_stats[i];
> + offset = bcmasp_stat_fixup_offset(intf, s);
> + switch (s->type) {
> + case BCMASP_STAT_SOFT:
> + continue;
> + case BCMASP_STAT_RX_EDPKT:
> + val = rx_edpkt_core_rl(intf->parent, offset);
> + break;
> + case BCMASP_STAT_RX_CTRL:
> + val = rx_ctrl_core_rl(intf->parent, offset);
> + break;
> + case BCMASP_STAT_RX_CTRL_PER_INTF:
> + offset += sizeof(u32) * intf->port;
> + val = rx_ctrl_core_rl(intf->parent, offset);
> + break;
> + }
> + p = (char *)(&intf->mib) + (i * sizeof(u32));
> + put_unaligned(val, (u32 *)p);

Hi Justin,

GCC 12.2.0, in a W=1 build, warns that val may be used uninitialised here.

I think that, in theory, that can occur if s->type doesn't match
any of the case statements above. Perhaps in practice that cannot occur.
But, perhaps it would be worth adding a default case with some suitable
handling.

In file included from drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c:4:
./include/asm-generic/unaligned.h: In function 'bcmasp_get_ethtool_stats':
./include/asm-generic/unaligned.h:19:19: warning: 'val' may be used uninitialized [-Wmaybe-uninitialized]
19 | __pptr->x = (val); \
| ^
drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c:125:29: note: 'val' was declared here
125 | u32 offset, val;
|

> + }
> +}

...