| Message ID | 20200906200548.18053-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eb2c6ca2db8c9cf112f0b604c1edfe21b5725f17 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | mwifiex: remove function pointer check | expand |
On Sun, Sep 06, 2020 at 01:05:48PM -0700, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > clang static analyzer reports this problem > > init.c:739:8: warning: Called function pointer > is null (null dereference) > ret = adapter->if_ops.check_fw_status( ... > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > In mwifiex_dnld_fw, there is an earlier check for check_fw_status(), > The check was introduced for usb support at the same time this > check in _mwifiex_fw_dpc() was made > > if (adapter->if_ops.dnld_fw) { > ret = adapter->if_ops.dnld_fw(adapter, &fw); > } else { > ret = mwifiex_dnld_fw(adapter, &fw); > } > > And a dnld_fw function initialized as part the usb's > mwifiex_if_ops. > > The other instances of mwifiex_if_ops for pci and sdio > both set check_fw_status. > > So the first check is not needed and can be removed. > > Fixes: 4daffe354366 ("mwifiex: add support for Marvell USB8797 chipset") > Signed-off-by: Tom Rix <trix@redhat.com> Indeed, on the surface, mwifiex_dnld_fw assumes that check_fw_status() cannot be NULL because it will always be called at the end of the function even if the first check is skipped. Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/net/wireless/marvell/mwifiex/init.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c > index 82d69bc3aaaf..f006a3d72b40 100644 > --- a/drivers/net/wireless/marvell/mwifiex/init.c > +++ b/drivers/net/wireless/marvell/mwifiex/init.c > @@ -695,14 +695,12 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter, > int ret; > u32 poll_num = 1; > > - if (adapter->if_ops.check_fw_status) { > - /* check if firmware is already running */ > - ret = adapter->if_ops.check_fw_status(adapter, poll_num); > - if (!ret) { > - mwifiex_dbg(adapter, MSG, > - "WLAN FW already running! Skip FW dnld\n"); > - return 0; > - } > + /* check if firmware is already running */ > + ret = adapter->if_ops.check_fw_status(adapter, poll_num); > + if (!ret) { > + mwifiex_dbg(adapter, MSG, > + "WLAN FW already running! Skip FW dnld\n"); > + return 0; > } > > /* check if we are the winner for downloading FW */ > -- > 2.18.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200906200548.18053-1-trix%40redhat.com.
trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > clang static analyzer reports this problem > > init.c:739:8: warning: Called function pointer > is null (null dereference) > ret = adapter->if_ops.check_fw_status( ... > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > In mwifiex_dnld_fw, there is an earlier check for check_fw_status(), > The check was introduced for usb support at the same time this > check in _mwifiex_fw_dpc() was made > > if (adapter->if_ops.dnld_fw) { > ret = adapter->if_ops.dnld_fw(adapter, &fw); > } else { > ret = mwifiex_dnld_fw(adapter, &fw); > } > > And a dnld_fw function initialized as part the usb's > mwifiex_if_ops. > > The other instances of mwifiex_if_ops for pci and sdio > both set check_fw_status. > > So the first check is not needed and can be removed. > > Fixes: 4daffe354366 ("mwifiex: add support for Marvell USB8797 chipset") > Signed-off-by: Tom Rix <trix@redhat.com> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Patch applied to wireless-drivers-next.git, thanks. eb2c6ca2db8c mwifiex: remove function pointer check
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c index 82d69bc3aaaf..f006a3d72b40 100644 --- a/drivers/net/wireless/marvell/mwifiex/init.c +++ b/drivers/net/wireless/marvell/mwifiex/init.c @@ -695,14 +695,12 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter, int ret; u32 poll_num = 1; - if (adapter->if_ops.check_fw_status) { - /* check if firmware is already running */ - ret = adapter->if_ops.check_fw_status(adapter, poll_num); - if (!ret) { - mwifiex_dbg(adapter, MSG, - "WLAN FW already running! Skip FW dnld\n"); - return 0; - } + /* check if firmware is already running */ + ret = adapter->if_ops.check_fw_status(adapter, poll_num); + if (!ret) { + mwifiex_dbg(adapter, MSG, + "WLAN FW already running! Skip FW dnld\n"); + return 0; } /* check if we are the winner for downloading FW */