Messages in this thread | | | Subject | Re: drivers/net/usb/pegasus.c:461:2: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores] | From | Pavel Skripkin <> | Date | Mon, 9 Aug 2021 23:37:08 +0300 |
| |
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 <lkp@intel.com> >> >> 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
| |