2021-08-09 10:49:47

by Chen, Rong A

[permalink] [raw]
Subject: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]


tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 85a90500f9a1717c4e142ce92e6c1cb1a339ec78
commit: 8a160e2e9aeb8318159b48701ad8a6e22274372d net: usb: pegasus:
Check the return value of get_geristers() and friends;
date: 4 days ago
:::::: branch date: 8 hours ago
:::::: commit date: 4 days ago
config: x86_64-randconfig-c001-20210808 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
41a6b50c25961addc04438b567ee1f4ef9e40f98)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a160e2e9aeb8318159b48701ad8a6e22274372d
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 8a160e2e9aeb8318159b48701ad8a6e22274372d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


clang-analyzer warnings: (new ones prefixed by >>)
^~~~
drivers/net/dsa/microchip/ksz_common.h:208:2: note: Taking false branch
if (!ret) {
^
drivers/net/dsa/microchip/ksz_common.h:215:2: note: Returning
without writing to '*val'
return ret;
^
drivers/net/dsa/microchip/ksz8795.c:464:2: note: Returning from
'ksz_read64'
ksz_read64(dev, regs[REG_IND_DATA_HI], data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:466:1: note: Returning without
writing to '*data'
}
^
drivers/net/dsa/microchip/ksz8795.c:588:2: note: Returning from
'ksz8_r_table'
ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:589:17: note: The left operand
of '>>' is a garbage value
data_hi = data >> 32;
~~~~ ^
drivers/net/dsa/microchip/ksz8795.c:692:38: warning: Assigned value
is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
dev->vlan_cache[addr + i].table[0] = (u16)data;
^
drivers/net/dsa/microchip/ksz8795.c:1404:6: note: Assuming field
'vlan_cache' is non-null
if (!dev->vlan_cache)
^~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:1404:2: note: Taking false branch
if (!dev->vlan_cache)
^
drivers/net/dsa/microchip/ksz8795.c:1408:6: note: 'ret' is 0
if (ret) {
^~~
drivers/net/dsa/microchip/ksz8795.c:1408:2: note: Taking false branch
if (ret) {
^
drivers/net/dsa/microchip/ksz8795.c:1444:14: note: Assuming the
condition is true
for (i = 0; i < (dev->num_vlans / 4); i++)
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:1444:2: note: Loop condition is
true. Entering loop body
for (i = 0; i < (dev->num_vlans / 4); i++)
^
drivers/net/dsa/microchip/ksz8795.c:1445:3: note: Calling
'ksz8_r_vlan_entries'
ksz8_r_vlan_entries(dev, i);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:684:2: note: 'data' declared
without an initial value
u64 data;
^~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:689:2: note: Calling 'ksz8_r_table'
ksz8_r_table(dev, TABLE_VLAN, addr, &data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:464:2: note: Calling 'ksz_read64'
ksz_read64(dev, regs[REG_IND_DATA_HI], data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz_common.h:208:6: note: Assuming 'ret'
is not equal to 0
if (!ret) {
^~~~
drivers/net/dsa/microchip/ksz_common.h:208:2: note: Taking false branch
if (!ret) {
^
drivers/net/dsa/microchip/ksz_common.h:215:2: note: Returning
without writing to '*val'
return ret;
^
drivers/net/dsa/microchip/ksz8795.c:464:2: note: Returning from
'ksz_read64'
ksz_read64(dev, regs[REG_IND_DATA_HI], data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:466:1: note: Returning without
writing to '*data'
}
^
drivers/net/dsa/microchip/ksz8795.c:689:2: note: Returning from
'ksz8_r_table'
ksz8_r_table(dev, TABLE_VLAN, addr, &data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:691:14: note: Assuming 'i' is <
field 'phy_port_cnt'
for (i = 0; i < dev->phy_port_cnt; i++) {
^~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:691:2: note: Loop condition is
true. Entering loop body
for (i = 0; i < dev->phy_port_cnt; i++) {
^
drivers/net/dsa/microchip/ksz8795.c:692:38: note: Assigned value is
garbage or undefined
dev->vlan_cache[addr + i].table[0] = (u16)data;
^ ~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:1592:8: warning: Excessive
padding in 'struct ksz_chip_data' (10 padding bytes, where 2 is
optimal). Optimal fields order: dev_name, num_vlans,
num_alus, num_statics, cpu_ports, port_cnt, chip_id,
consider reordering the fields or adding explicit padding members
[clang-analyzer-optin.performance.Padding]
struct ksz_chip_data {
~~~~~~~^~~~~~~~~~~~~~~
drivers/net/dsa/microchip/ksz8795.c:1592:8: note: Excessive padding
in 'struct ksz_chip_data' (10 padding bytes, where 2 is optimal).
Optimal fields order: dev_name, num_vlans, num_alus, num_statics,
cpu_ports, port_cnt, chip_id, consider reordering the fields or adding
explicit padding members
struct ksz_chip_data {
~~~~~~~^~~~~~~~~~~~~~~
Suppressed 11 warnings (11 in non-user code).
Use -header-filter=.* to display errors from all non-system headers.
Use -system-headers to display errors from system headers as well.
10 warnings generated.
Suppressed 10 warnings (10 in non-user code).
Use -header-filter=.* to display errors from all non-system headers.
Use -system-headers to display errors from system headers as well.
12 warnings generated.
>> drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
ret = set_registers(pegasus, EthCtrl0, 3, data);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/pegasus.c:461:2: note: Value stored to 'ret' is
never read
ret = set_registers(pegasus, EthCtrl0, 3, data);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/pegasus.c:778:18: warning: The left operand of '>>'
is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
interval = data >> 8;
^
drivers/net/usb/pegasus.c:1157:2: note: Taking false branch
if (pegasus_blacklisted(dev))
^
drivers/net/usb/pegasus.c:1161:6: note: Assuming 'net' is non-null
if (!net)
^~~~
drivers/net/usb/pegasus.c:1161:2: note: Taking false branch
if (!net)
^
drivers/net/usb/pegasus.c:1168:6: note: 'res' is >= 0
if (res < 0) {
^~~
drivers/net/usb/pegasus.c:1168:2: note: Taking false branch
if (res < 0) {
^
drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
Exiting loop
INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
^
include/linux/workqueue.h:272:2: note: expanded from macro
'INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
include/linux/workqueue.h:257:3: note: expanded from macro
'__INIT_DELAYED_WORK'
INIT_WORK(&(_work)->work, (_func));
\
^
include/linux/workqueue.h:250:2: note: expanded from macro 'INIT_WORK'
__INIT_WORK((_work), (_func), 0)
^
include/linux/workqueue.h:230:2: note: expanded from macro '__INIT_WORK'
do {
\
^
drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
Exiting loop
INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
^
include/linux/workqueue.h:272:2: note: expanded from macro
'INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
include/linux/workqueue.h:258:3: note: expanded from macro
'__INIT_DELAYED_WORK'
__init_timer(&(_work)->timer,
\
^
include/linux/timer.h:113:2: note: expanded from macro '__init_timer'
do {
\
^
drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
Exiting loop
INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
^
include/linux/workqueue.h:272:2: note: expanded from macro
'INIT_DELAYED_WORK'
__INIT_DELAYED_WORK(_work, _func, 0)
^
include/linux/workqueue.h:256:2: note: expanded from macro
'__INIT_DELAYED_WORK'
do {
\
^
drivers/net/usb/pegasus.c:1194:2: note: Calling 'get_interrupt_interval'
get_interrupt_interval(pegasus);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/pegasus.c:774:2: note: 'data' declared without an
initial value
u16 data;
^~~~~~~~
drivers/net/usb/pegasus.c:777:2: note: Calling 'read_eprom_word'
read_eprom_word(pegasus, 4, &data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/pegasus.c:279:2: note: Loop condition is true.
Entering loop body
for (i = 0; i < REG_TIMEOUT; i++) {
^
drivers/net/usb/pegasus.c:281:7: note: Assuming 'ret' is < 0
if (ret < 0)
^~~~~~~
drivers/net/usb/pegasus.c:281:3: note: Taking true branch
if (ret < 0)
^
drivers/net/usb/pegasus.c:282:4: note: Control jumps to line 298
goto fail;
^
drivers/net/usb/pegasus.c:298:2: note: Taking false branch
netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
^
include/linux/netdevice.h:5352:2: note: expanded from macro 'netif_dbg'
if (netif_msg_##type(priv)) \
^
drivers/net/usb/pegasus.c:298:2: note: Loop condition is false.
Exiting loop
netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
^
include/linux/netdevice.h:5350:57: note: expanded from macro 'netif_dbg'
#define netif_dbg(priv, type, netdev, format, args...) \
^
drivers/net/usb/pegasus.c:299:2: note: Returning without writing to
'*retdata'
return ret;
^
drivers/net/usb/pegasus.c:777:2: note: Returning from 'read_eprom_word'
read_eprom_word(pegasus, 4, &data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/pegasus.c:778:18: note: The left operand of '>>' is
a garbage value
interval = data >> 8;

vim +/ret +461 drivers/net/usb/pegasus.c

^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 439
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
440 static int enable_net_traffic(struct net_device *dev, struct
usb_device *usb)
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 441 {
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
442 pegasus_t *pegasus = netdev_priv(dev);
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
443 int ret;
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
444 __u16 linkpart;
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
445 __u8 data[4];
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 446
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
447 ret = read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
448 if (ret < 0)
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
449 goto fail;
1a8deec09d12c1 drivers/net/usb/pegasus.c Petko Manolov 2016-04-27
450 data[0] = 0xc8; /* TX & RX enable, append status, no CRC */
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
451 data[1] = 0;
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
452 if (linkpart & (ADVERTISE_100FULL | ADVERTISE_10FULL))
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
453 data[1] |= 0x20; /* set full duplex */
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
454 if (linkpart & (ADVERTISE_100FULL | ADVERTISE_100HALF))
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
455 data[1] |= 0x10; /* set 100 Mbps */
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
456 if (mii_mode)
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
457 data[1] = 0;
681f16232c49de drivers/net/usb/pegasus.c Dan Carpenter 2011-12-23
458 data[2] = loopback ? 0x09 : 0x01;
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 459
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
460 memcpy(pegasus->eth_regs, data, sizeof(data));
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
@461 ret = set_registers(pegasus, EthCtrl0, 3, data);
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 462
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
463 if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
efafe6fb72b2bb drivers/usb/net/pegasus.c Malte Doersam 2006-01-28
464 usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
465 usb_dev_id[pegasus->dev_index].vendor == VENDOR_DLINK) {
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
466 u16 auxmode;
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
467 ret = read_mii_word(pegasus, 0, 0x1b, &auxmode);
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
468 if (ret < 0)
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
469 goto fail;
2bd647018fe1b2 drivers/net/usb/pegasus.c Petko Manolov 2013-04-25
470 auxmode |= 4;
2bd647018fe1b2 drivers/net/usb/pegasus.c Petko Manolov 2013-04-25
471 write_mii_word(pegasus, 0, 0x1b, &auxmode);
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 472 }
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 473
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
474 return 0;
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03 475
fail:
8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
476 netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
4a1728a28a193a drivers/usb/net/pegasus.c Petko Manolov 2005-11-15
477 return ret;
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 478 }
^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 479
:::::: The code at line 461 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
.config.gz (36.49 kB)
Attached Message Part (154.00 B)
Download all attachments

2021-08-09 12:06:53

by Pavel Skripkin

[permalink] [raw]
Subject: Re: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]

On 8/9/21 1:37 PM, kernel test robot wrote:
>
> tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 85a90500f9a1717c4e142ce92e6c1cb1a339ec78
> commit: 8a160e2e9aeb8318159b48701ad8a6e22274372d net: usb: pegasus:
> Check the return value of get_geristers() and friends;
> date: 4 days ago
> :::::: branch date: 8 hours ago
> :::::: commit date: 4 days ago
> config: x86_64-randconfig-c001-20210808 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> 41a6b50c25961addc04438b567ee1f4ef9e40f98)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> #
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a160e2e9aeb8318159b48701ad8a6e22274372d
> git remote add linus
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 8a160e2e9aeb8318159b48701ad8a6e22274372d
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> ARCH=x86_64 clang-analyzer
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>

[snip]

>>> drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
> ret = set_registers(pegasus, EthCtrl0, 3, data);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/usb/pegasus.c:461:2: note: Value stored to 'ret' is
> never read
> ret = set_registers(pegasus, EthCtrl0, 3, data);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/usb/pegasus.c:778:18: warning: The left operand of '>>'
> is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
> interval = data >> 8;
> ^

This is fixed by commit af35fc37354c ("net: pegasus: fix uninit-value in
get_interrupt_interval")


> drivers/net/usb/pegasus.c:1157:2: note: Taking false branch
> if (pegasus_blacklisted(dev))
> ^
> drivers/net/usb/pegasus.c:1161:6: note: Assuming 'net' is non-null
> if (!net)
> ^~~~
> drivers/net/usb/pegasus.c:1161:2: note: Taking false branch
> if (!net)
> ^
> drivers/net/usb/pegasus.c:1168:6: note: 'res' is >= 0
> if (res < 0) {
> ^~~
> drivers/net/usb/pegasus.c:1168:2: note: Taking false branch
> if (res < 0) {
> ^
> drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
> Exiting loop
> INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
> ^
> include/linux/workqueue.h:272:2: note: expanded from macro
> 'INIT_DELAYED_WORK'
> __INIT_DELAYED_WORK(_work, _func, 0)
> ^
> include/linux/workqueue.h:257:3: note: expanded from macro
> '__INIT_DELAYED_WORK'
> INIT_WORK(&(_work)->work, (_func));
> \
> ^
> include/linux/workqueue.h:250:2: note: expanded from macro 'INIT_WORK'
> __INIT_WORK((_work), (_func), 0)
> ^
> include/linux/workqueue.h:230:2: note: expanded from macro '__INIT_WORK'
> do {
> \
> ^
> drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
> Exiting loop
> INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
> ^
> include/linux/workqueue.h:272:2: note: expanded from macro
> 'INIT_DELAYED_WORK'
> __INIT_DELAYED_WORK(_work, _func, 0)
> ^
> include/linux/workqueue.h:258:3: note: expanded from macro
> '__INIT_DELAYED_WORK'
> __init_timer(&(_work)->timer,
> \
> ^
> include/linux/timer.h:113:2: note: expanded from macro '__init_timer'
> do {
> \
> ^
> drivers/net/usb/pegasus.c:1175:2: note: Loop condition is false.
> Exiting loop
> INIT_DELAYED_WORK(&pegasus->carrier_check, check_carrier);
> ^
> include/linux/workqueue.h:272:2: note: expanded from macro
> 'INIT_DELAYED_WORK'
> __INIT_DELAYED_WORK(_work, _func, 0)
> ^
> include/linux/workqueue.h:256:2: note: expanded from macro
> '__INIT_DELAYED_WORK'
> do {
> \
> ^
> drivers/net/usb/pegasus.c:1194:2: note: Calling 'get_interrupt_interval'
> get_interrupt_interval(pegasus);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/usb/pegasus.c:774:2: note: 'data' declared without an
> initial value
> u16 data;
> ^~~~~~~~
> drivers/net/usb/pegasus.c:777:2: note: Calling 'read_eprom_word'
> read_eprom_word(pegasus, 4, &data);

This is fixed by commit af35fc37354c ("net: pegasus: fix uninit-value in
get_interrupt_interval")


> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/net/usb/pegasus.c:279:2: note: Loop condition is true.
> Entering loop body
> for (i = 0; i < REG_TIMEOUT; i++) {
> ^
> drivers/net/usb/pegasus.c:281:7: note: Assuming 'ret' is < 0
> if (ret < 0)
> ^~~~~~~
> drivers/net/usb/pegasus.c:281:3: note: Taking true branch
> if (ret < 0)
> ^
> drivers/net/usb/pegasus.c:282:4: note: Control jumps to line 298
> goto fail;
> ^
> drivers/net/usb/pegasus.c:298:2: note: Taking false branch
> netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
> ^
> include/linux/netdevice.h:5352:2: note: expanded from macro 'netif_dbg'
> if (netif_msg_##type(priv)) \
> ^
> drivers/net/usb/pegasus.c:298:2: note: Loop condition is false.
> Exiting loop
> netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
> ^
> include/linux/netdevice.h:5350:57: note: expanded from macro 'netif_dbg'
> #define netif_dbg(priv, type, netdev, format, args...) \
> ^
> drivers/net/usb/pegasus.c:299:2: note: Returning without writing to
> '*retdata'
> return ret;
> ^
> drivers/net/usb/pegasus.c:777:2: note: Returning from 'read_eprom_word'
> read_eprom_word(pegasus, 4, &data);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/usb/pegasus.c:778:18: note: The left operand of '>>' is
> a garbage value
> interval = data >> 8;

This is fixed by commit af35fc37354c ("net: pegasus: fix uninit-value in
get_interrupt_interval")


>
> vim +/ret +461 drivers/net/usb/pegasus.c
>
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 439
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 440 static int enable_net_traffic(struct net_device *dev, struct
> usb_device *usb)
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 441 {
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 442 pegasus_t *pegasus = netdev_priv(dev);
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 443 int ret;
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 444 __u16 linkpart;
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 445 __u8 data[4];
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 446
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 447 ret = read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 448 if (ret < 0)
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 449 goto fail;
> 1a8deec09d12c1 drivers/net/usb/pegasus.c Petko Manolov 2016-04-27
> 450 data[0] = 0xc8; /* TX & RX enable, append status, no CRC */
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 451 data[1] = 0;
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 452 if (linkpart & (ADVERTISE_100FULL | ADVERTISE_10FULL))
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 453 data[1] |= 0x20; /* set full duplex */
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 454 if (linkpart & (ADVERTISE_100FULL | ADVERTISE_100HALF))
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 455 data[1] |= 0x10; /* set 100 Mbps */
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 456 if (mii_mode)
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 457 data[1] = 0;
> 681f16232c49de drivers/net/usb/pegasus.c Dan Carpenter 2011-12-23
> 458 data[2] = loopback ? 0x09 : 0x01;
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 459
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 460 memcpy(pegasus->eth_regs, data, sizeof(data));
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> @461 ret = set_registers(pegasus, EthCtrl0, 3, data);
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 462
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 463 if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
> efafe6fb72b2bb drivers/usb/net/pegasus.c Malte Doersam 2006-01-28
> 464 usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 465 usb_dev_id[pegasus->dev_index].vendor == VENDOR_DLINK) {
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16
> 466 u16 auxmode;
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 467 ret = read_mii_word(pegasus, 0, 0x1b, &auxmode);
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 468 if (ret < 0)
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 469 goto fail;
> 2bd647018fe1b2 drivers/net/usb/pegasus.c Petko Manolov 2013-04-25
> 470 auxmode |= 4;
> 2bd647018fe1b2 drivers/net/usb/pegasus.c Petko Manolov 2013-04-25
> 471 write_mii_word(pegasus, 0, 0x1b, &auxmode);
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 472 }
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 473
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 474 return 0;
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03 475
> fail:
> 8a160e2e9aeb83 drivers/net/usb/pegasus.c Petko Manolov 2021-08-03
> 476 netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
> 4a1728a28a193a drivers/usb/net/pegasus.c Petko Manolov 2005-11-15
> 477 return ret;
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 478 }
> ^1da177e4c3f41 drivers/usb/net/pegasus.c Linus Torvalds 2005-04-16 479
> :::::: The code at line 461 was first introduced by commit
> :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
>
> :::::: TO: Linus Torvalds <[email protected]>
> :::::: CC: Linus Torvalds <[email protected]>
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>


With regards,
Pavel Skripkin

2021-08-09 12:07:59

by Pavel Skripkin

[permalink] [raw]
Subject: Re: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]

On 8/9/21 1:37 PM, kernel test robot wrote:
>
> tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 85a90500f9a1717c4e142ce92e6c1cb1a339ec78
> commit: 8a160e2e9aeb8318159b48701ad8a6e22274372d net: usb: pegasus:
> Check the return value of get_geristers() and friends;
> date: 4 days ago
> :::::: branch date: 8 hours ago
> :::::: commit date: 4 days ago
> config: x86_64-randconfig-c001-20210808 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> 41a6b50c25961addc04438b567ee1f4ef9e40f98)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> #
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a160e2e9aeb8318159b48701ad8a6e22274372d
> git remote add linus
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 8a160e2e9aeb8318159b48701ad8a6e22274372d
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> ARCH=x86_64 clang-analyzer
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>

Hi, @Petko!

For you not to scan all these warnings:

>>> drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
> ret = set_registers(pegasus, EthCtrl0, 3, data);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/usb/pegasus.c:461:2: note: Value stored to 'ret' is
> never read
> ret = set_registers(pegasus, EthCtrl0, 3, data);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is the real bug, I think. Can be fixed like this:

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 22353bab76c8..f2b8891c7b36 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -459,6 +459,8 @@ static int enable_net_traffic(struct net_device
*dev, struct usb_device *usb)

memcpy(pegasus->eth_regs, data, sizeof(data));
ret = set_registers(pegasus, EthCtrl0, 3, data);
+ if (ret < 0)
+ goto fail;

if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||


It was caused by our last refactoring: enable_net_traffic() now returns
0 on success and this ret is never checked.


With regards,
Pavel Skripkin

2021-08-09 18:38:59

by Petko Manolov

[permalink] [raw]
Subject: Re: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]

On 21-08-09 14:06:11, Pavel Skripkin wrote:
> On 8/9/21 1:37 PM, kernel test robot wrote:
> >
> > tree:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 85a90500f9a1717c4e142ce92e6c1cb1a339ec78
> > commit: 8a160e2e9aeb8318159b48701ad8a6e22274372d net: usb: pegasus:
> > Check the return value of get_geristers() and friends;
> > date: 4 days ago
> > :::::: branch date: 8 hours ago
> > :::::: commit date: 4 days ago
> > config: x86_64-randconfig-c001-20210808 (attached as .config)
> > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> > 41a6b50c25961addc04438b567ee1f4ef9e40f98)
> > reproduce (this is a W=1 build):
> > wget
> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # install x86_64 cross compiling tool for clang build
> > # apt-get install binutils-x86-64-linux-gnu
> > #
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a160e2e9aeb8318159b48701ad8a6e22274372d
> > git remote add linus
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git fetch --no-tags linus master
> > git checkout 8a160e2e9aeb8318159b48701ad8a6e22274372d
> > # save the attached .config to linux build tree
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> > ARCH=x86_64 clang-analyzer
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
>
> Hi, @Petko!
>
> For you not to scan all these warnings:
>
> > > > drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
> > ret = set_registers(pegasus, EthCtrl0, 3, data);
> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/net/usb/pegasus.c:461:2: note: Value stored to 'ret' is
> > never read
> > ret = set_registers(pegasus, EthCtrl0, 3, data);
> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This is the real bug, I think. Can be fixed like this:
>
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 22353bab76c8..f2b8891c7b36 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -459,6 +459,8 @@ static int enable_net_traffic(struct net_device *dev,
> struct usb_device *usb)
>
> memcpy(pegasus->eth_regs, data, sizeof(data));
> ret = set_registers(pegasus, EthCtrl0, 3, data);
> + if (ret < 0)
> + goto fail;
>
> if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
> usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
>
>
> It was caused by our last refactoring: enable_net_traffic() now returns 0 on
> success and this ret is never checked.

I'd rather remove the 'ret = ' part and leave set_registers() alone. If this
particular write operation fail, it doesn't mean the adapter won't work at all.
Perhaps it won't be the most optimal mode, but it will work. There are some
legal checks after set_registers() that also make sense to pass. So the patch i
suggest looks like:


diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 652e9fcf0b77..49cfc720d78f 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -433,7 +433,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
data[2] = loopback ? 0x09 : 0x01;

memcpy(pegasus->eth_regs, data, sizeof(data));
- ret = set_registers(pegasus, EthCtrl0, 3, data);
+ set_registers(pegasus, EthCtrl0, 3, data);

if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||


cheers,
Petko

2021-08-09 23:01:38

by Pavel Skripkin

[permalink] [raw]
Subject: Re: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]

On 8/9/21 9:36 PM, Petko Manolov wrote:
> On 21-08-09 14:06:11, Pavel Skripkin wrote:
>> On 8/9/21 1:37 PM, kernel test robot wrote:
>> >
>> > tree:
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> > head: 85a90500f9a1717c4e142ce92e6c1cb1a339ec78
>> > commit: 8a160e2e9aeb8318159b48701ad8a6e22274372d net: usb: pegasus:
>> > Check the return value of get_geristers() and friends;
>> > date: 4 days ago
>> > :::::: branch date: 8 hours ago
>> > :::::: commit date: 4 days ago
>> > config: x86_64-randconfig-c001-20210808 (attached as .config)
>> > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
>> > 41a6b50c25961addc04438b567ee1f4ef9e40f98)
>> > reproduce (this is a W=1 build):
>> > wget
>> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>> > -O ~/bin/make.cross
>> > chmod +x ~/bin/make.cross
>> > # install x86_64 cross compiling tool for clang build
>> > # apt-get install binutils-x86-64-linux-gnu
>> > #
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8a160e2e9aeb8318159b48701ad8a6e22274372d
>> > git remote add linus
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> > git fetch --no-tags linus master
>> > git checkout 8a160e2e9aeb8318159b48701ad8a6e22274372d
>> > # save the attached .config to linux build tree
>> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
>> > ARCH=x86_64 clang-analyzer
>> > If you fix the issue, kindly add following tag as appropriate
>> > Reported-by: kernel test robot <[email protected]>
>>
>> Hi, @Petko!
>>
>> For you not to scan all these warnings:
>>
>> > > > drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
>> > ret = set_registers(pegasus, EthCtrl0, 3, data);
>> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > drivers/net/usb/pegasus.c:461:2: note: Value stored to 'ret' is
>> > never read
>> > ret = set_registers(pegasus, EthCtrl0, 3, data);
>> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This is the real bug, I think. Can be fixed like this:
>>
>> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
>> index 22353bab76c8..f2b8891c7b36 100644
>> --- a/drivers/net/usb/pegasus.c
>> +++ b/drivers/net/usb/pegasus.c
>> @@ -459,6 +459,8 @@ static int enable_net_traffic(struct net_device *dev,
>> struct usb_device *usb)
>>
>> memcpy(pegasus->eth_regs, data, sizeof(data));
>> ret = set_registers(pegasus, EthCtrl0, 3, data);
>> + if (ret < 0)
>> + goto fail;
>>
>> if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
>> usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
>>
>>
>> It was caused by our last refactoring: enable_net_traffic() now returns 0 on
>> success and this ret is never checked.
>
> I'd rather remove the 'ret = ' part and leave set_registers() alone. If this
> particular write operation fail, it doesn't mean the adapter won't work at all.
> Perhaps it won't be the most optimal mode, but it will work. There are some
> legal checks after set_registers() that also make sense to pass. So the patch i
> suggest looks like:
>
>
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 652e9fcf0b77..49cfc720d78f 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -433,7 +433,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
> data[2] = loopback ? 0x09 : 0x01;
>
> memcpy(pegasus->eth_regs, data, sizeof(data));
> - ret = set_registers(pegasus, EthCtrl0, 3, data);
> + set_registers(pegasus, EthCtrl0, 3, data);
>
> if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
> usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
>

It works. I am not aware of device specifics, so I decided to handle the
error instead of ignoring.

Will you take care of posting this patch, or I can do it with
Suggested-by tag?



With regards,
Pavel Skripkin