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

[Simon Horman]

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;
       |  

> +	}
> +}

...