staging: rtl8192e: Fix comparison to NULL of variable rf_set_sens
PATCH v3 1/1 staging: rtl8192e: Fix comparison to NULL of variable rf_set_sens
Functions _rtl92e_wx_get_sens and _rtl92e_wx_set_sens depend on the function rf_set_sens, which is declared but never defined. Hence calling this function will cause an oops.
Because there is no definition of the function priv->rf_set_sens will always be NULL.
As a result _rtl92e_wx_set_sens and _rtl92e_wx_get_sens will always return -1.
Hence,
* Removed function definition rf_set_sens
* Removed usage of variable priv->rf_set_sens
* Removed functions _rtl92e_wx_get_sens and _rtl92e_wx_set_sens
* Cleaned up the variables sens and max_sens used in these functions
---
Changed in v3:
- Fixed description to fit within 75 chars
- Added suggested tags
as Suggested by: Philipp Hortmann <[email protected]>
Changed in v2:
- Removed function definition rf_set_sens
- Removed usage of variable priv->rf_set_sens
- Removed functions _rtl92e_wx_get_sens and _rtl92e_wx_set_sens
- Cleaned up the variables sens and max_sens used in these functions
--- drivers/staging/rtl8192e/rtl8192e/rtl_core.h | 3 -- drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 45 -------------------- 2 files changed, 48 deletions(-)
drivers/staging/rtl8192e/rtl8192e/rtl_core.h +0 -3
@@ -234,7 +234,6 @@ struct r8192_priv {struct rt_stats stats;struct iw_statistics wstats; -short (*rf_set_sens)(struct net_device *dev,short sens);u8 (*rf_set_chan)(struct net_device *dev,u8 ch);struct rx_desc *rx_ring[MAX_RX_QUEUE ];
@@ -274,8 +273,6 @@ struct r8192_priv {short promisc;short chan; -short sens; -short max_sens;bool ps_force;u32 irq_mask[2 ];
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0 -45
63 lines not on this page · raw message
Signature · 1 line
2.34.1
Greg Kroah-Hartman
maintainerQuoted 3 lines · Yogesh Hegde
Functions
_rtl92e_wx_get_sensand_rtl92e_wx_set_sensdepend on the functionrf_set_sens, which is declared but never defined. Hence calling this function will cause an oops.
So can an oops happen? I'm confused.
Quoted 2 lines
Because there is no definition of the function
priv->rf_set_senswill always be NULL.
Ok, but is it used? I'm confused.
Quoted 3 lines
As a result
_rtl92e_wx_set_sensand_rtl92e_wx_get_senswill always return -1.
So it does work? Or it doesn't? What about the oops?
Quoted 6 lines
Hence,
* Removed function definitionrf_set_sens
* Removed usage of variablepriv->rf_set_sens
* Removed functions_rtl92e_wx_get_sensand_rtl92e_wx_set_sens
* Cleaned up the variables sens andmax_sensused in these functions
"Hence" is an odd way to write this.
And when you list things in a changelog, that usually means you want to have multiple patches, right?
Quoted 3 lines
Suggested-by Philipp Hortmann <[email protected]>Signed-off-by Yogesh Hegde <[email protected]>
No blank line between these please.
Quoted 100 lines
---
Changed in v3:
- Fixed description to fit within 75 chars
- Added suggested tagsas Suggested by: Philipp Hortmann <[email protected]>
Changed in v2:
- Removed function definitionrf_set_sens
- Removed usage of variablepriv->rf_set_sens
- Removed functions_rtl92e_wx_get_sensand_rtl92e_wx_set_sens
- Cleaned up the variables sens andmax_sensused in these functions--- drivers/staging/rtl8192e/rtl8192e/rtl_core.h | 3 -- drivers/staging/rtl8192e/rtl8192e/rtl_wx.c | 45 -------------------- 2 files changed, 48 deletions(-)drivers/staging/rtl8192e/rtl8192e/rtl_core.h +0
-3@@ -234,7 +234,6 @@ struct r8192_priv {struct rt_stats stats;struct iw_statistics wstats; -short (*rf_set_sens)(struct net_device *dev,short sens);u8 (*rf_set_chan)(struct net_device *dev,u8 ch);struct rx_desc *rx_ring[MAX_RX_QUEUE ];@@ -274,8 +273,6 @@ struct r8192_priv {short promisc;short chan; -short sens; -short max_sens;bool ps_force;u32 irq_mask[2 ];drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0
-43@@ -311,10 +311,6 @@ static int _rtl92e_wx_get_range(struct net_device *dev,/* ~130 Mb/s real (802.11n) */ range->throughput =130 *1000 *1000 ; -if (priv->rf_set_sens !=NULL ) -/* signal level threshold range */ - range->sensitivity = priv->max_sens; - range->max_qual.qual =100 ; range->max_qual.level =0 ; range->max_qual.noise =0 ;@@ -807,45 +803,6 @@ static int _rtl92e_wx_get_retry(struct net_device *dev,return 0 ; } -static int _rtl92e_wx_get_sens (struct net_device *dev, -struct iw_request_info *info, -union iwreq_data *wrqu,char *extra) -{ -struct r8192_priv *priv =rtllib_priv (dev); - -if (priv->rf_set_sens ==NULL ) -return -1 ;/* we have not this support for this radio */ - wrqu->sens.value = priv->sens; -return 0 ; -} - -static int _rtl92e_wx_set_sens (struct net_device *dev, -struct iw_request_info *info, -union iwreq_data *wrqu,char *extra) -{ -struct r8192_priv *priv =rtllib_priv (dev); - -short err =0 ; - -if (priv->hw_radio_off) -return 0 ; - -mutex_lock (&priv->wx_mutex); -if (priv->rf_set_sens ==NULL ) { - err =-1 ;/* we have not this support for this radio */ - goto exit; - } -if (priv->rf_set_sens (dev, wrqu->sens.value) ==0 ) - priv->sens = wrqu->sens.value; -else - err = -EINVAL ; - -exit: -mutex_unlock (&priv->wx_mutex); - -return err; -} -static int _rtl92e_wx_set_encode_ext (struct net_device *dev,struct iw_request_info *info,union iwreq_data *wrqu,char *extra)
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c static iw_handler r8192_wx_handlers[] = { · line 1070
Quoted 5 lines
drivers/staging/rtl8192e/rtl8192e/rtl_wx.c +0
-2@@ -1066,8 +1023,6 @@ static iw_handler r8192_wx_handlers[] = { [IW_IOCTL(SIOCGIWFREQ)] = _rtl92e_wx_get_freq, [IW_IOCTL(SIOCSIWMODE)] = _rtl92e_wx_set_mode, [IW_IOCTL(SIOCGIWMODE)] = _rtl92e_wx_get_mode, - [IW_IOCTL(SIOCSIWSENS)] = _rtl92e_wx_set_sens, - [IW_IOCTL(SIOCGIWSENS)] = _rtl92e_wx_get_sens,
You are removing 2 ioctls, so at the minimum, this should be 2 different patches. Please make a patch series here and read up on how to write a bit better of a changelog text to make it more obvious what is happening here.
thanks,
greg k-h