Hi, David
This is a good suggestion and I will adopt it, send v8. Thanks!
> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Tuesday, May 24, 2022 5:04 PM
> To: Zhang, RobinX <robinx.zh...@intel.com>
> Cc: dev <dev@dpdk.org>; Thomas Monjalon <tho...@monjalon.net>;
> Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Liu, KevinX
> <kevinx....@intel.com>
> Subject: Re: [PATCH v7 5/5] ethdev: format module EEPROM for SFF-8636
>
> Hello,
>
> I see a lot of switch / case where tables could do the job.
> In any case, I'll focus only on one part of the code, that triggers a build
> warning witch clang 14 (+ ASan):
>
> On Tue, May 24, 2022 at 8:31 AM Robin Zhang <robinx.zh...@intel.com>
> wrote:
> > + /* Channel Specific Data */
> > + for (i = 0; i < MAX_CHANNEL_NUM; i++) {
> > + uint8_t rx_power_offset, tx_bias_offset;
> > + uint8_t tx_power_offset;
> > +
> > + switch (i) {
> > + case 0:
> > + rx_power_offset = SFF_8636_RX_PWR_1_OFFSET;
> > + tx_power_offset = SFF_8636_TX_PWR_1_OFFSET;
> > + tx_bias_offset = SFF_8636_TX_BIAS_1_OFFSET;
> > + break;
> > + case 1:
> > + rx_power_offset = SFF_8636_RX_PWR_2_OFFSET;
> > + tx_power_offset = SFF_8636_TX_PWR_2_OFFSET;
> > + tx_bias_offset = SFF_8636_TX_BIAS_2_OFFSET;
> > + break;
> > + case 2:
> > + rx_power_offset = SFF_8636_RX_PWR_3_OFFSET;
> > + tx_power_offset = SFF_8636_TX_PWR_3_OFFSET;
> > + tx_bias_offset = SFF_8636_TX_BIAS_3_OFFSET;
> > + break;
> > + case 3:
> > + rx_power_offset = SFF_8636_RX_PWR_4_OFFSET;
> > + tx_power_offset = SFF_8636_TX_PWR_4_OFFSET;
> > + tx_bias_offset = SFF_8636_TX_BIAS_4_OFFSET;
> > + break;
> > + }
> > + sd->scd[i].bias_cur = OFFSET_TO_U16(tx_bias_offset);
> > + sd->scd[i].rx_power = OFFSET_TO_U16(rx_power_offset);
> > + sd->scd[i].tx_power = OFFSET_TO_U16(tx_power_offset);
> > + }
>
> [15/442] Compiling C object lib/librte_ethdev.a.p/ethdev_sff_8636.c.o
> In file included from ../lib/ethdev/sff_8636.c:13:
> ../lib/ethdev/sff_common.h: In function ‘sff_8636_show_all’:
> ../lib/ethdev/sff_common.h:101:22: warning: ‘tx_power_offset’ may be
> used uninitialized [-Wmaybe-uninitialized]
> 101 | (data[offset] << 8 | data[(offset) + 1])
> | ^
> ../lib/ethdev/sff_8636.c:621:25: note: ‘tx_power_offset’ was declared here
> 621 | uint8_t tx_power_offset;
> | ^~~~~~~~~~~~~~~
> ../lib/ethdev/sff_common.h:101:22: warning: ‘tx_bias_offset’ may be used
> uninitialized [-Wmaybe-uninitialized]
> 101 | (data[offset] << 8 | data[(offset) + 1])
> | ^
> ../lib/ethdev/sff_8636.c:620:42: note: ‘tx_bias_offset’ was declared here
> 620 | uint8_t rx_power_offset, tx_bias_offset;
> | ^~~~~~~~~~~~~~
> ../lib/ethdev/sff_common.h:101:22: warning: ‘rx_power_offset’ may be
> used uninitialized [-Wmaybe-uninitialized]
> 101 | (data[offset] << 8 | data[(offset) + 1])
> | ^
> ../lib/ethdev/sff_8636.c:620:25: note: ‘rx_power_offset’ was declared here
> 620 | uint8_t rx_power_offset, tx_bias_offset;
> | ^~~~~~~~~~~~~~~
> [268/268] Linking target app/test/dpdk-test
>
>
> This is a false positive.
> This can be avoided using some tables:
>
> + const uint8_t rx_power_offset[MAX_CHANNEL_NUM] = {
> + SFF_8636_RX_PWR_1_OFFSET,
> + SFF_8636_RX_PWR_2_OFFSET,
> + SFF_8636_RX_PWR_3_OFFSET,
> + SFF_8636_RX_PWR_4_OFFSET,
> + };
> + const uint8_t tx_power_offset[MAX_CHANNEL_NUM] = {
> + SFF_8636_TX_PWR_1_OFFSET,
> + SFF_8636_TX_PWR_2_OFFSET,
> + SFF_8636_TX_PWR_3_OFFSET,
> + SFF_8636_TX_PWR_4_OFFSET,
> + };
> + const uint8_t tx_bias_offset[MAX_CHANNEL_NUM] = {
> + SFF_8636_TX_BIAS_1_OFFSET,
> + SFF_8636_TX_BIAS_2_OFFSET,
> + SFF_8636_TX_BIAS_3_OFFSET,
> + SFF_8636_TX_BIAS_4_OFFSET,
> + };
> + sd->scd[i].bias_cur = OFFSET_TO_U16(tx_bias_offset[i]);
> + sd->scd[i].rx_power = OFFSET_TO_U16(rx_power_offset[i]);
> + sd->scd[i].tx_power = OFFSET_TO_U16(tx_power_offset[i]);
>
>
> --
> David Marchand
