* [PATCH 1/3] staging: rtl8188eu: use IW_HANDLER to declare wext handlers
@ 2021-05-29 12:13 Martin Kaiser
2021-05-29 12:13 ` [PATCH 2/3] staging: rtl8188eu: remove dummy " Martin Kaiser
2021-05-29 12:13 ` [PATCH 3/3] staging: rtl8188eu: remove dummy setfreq handler Martin Kaiser
0 siblings, 2 replies; 5+ messages in thread
From: Martin Kaiser @ 2021-05-29 12:13 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman
Cc: linux-staging, kernel-janitors, linux-kernel, Martin Kaiser
Use the IW_HANDLER macro to declare the handler functions for
wext ioctls. We don't have to skip unused ioctl numbers manually.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
.../staging/rtl8188eu/os_dep/ioctl_linux.c | 97 ++++++++-----------
1 file changed, 41 insertions(+), 56 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 9dacdd595b63..a23ccda6fefe 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -2899,62 +2899,47 @@ static int rtw_wx_set_priv(struct net_device *dev,
}
static iw_handler rtw_handlers[] = {
- NULL, /* SIOCSIWCOMMIT */
- rtw_wx_get_name, /* SIOCGIWNAME */
- dummy, /* SIOCSIWNWID */
- dummy, /* SIOCGIWNWID */
- rtw_wx_set_freq, /* SIOCSIWFREQ */
- rtw_wx_get_freq, /* SIOCGIWFREQ */
- rtw_wx_set_mode, /* SIOCSIWMODE */
- rtw_wx_get_mode, /* SIOCGIWMODE */
- dummy, /* SIOCSIWSENS */
- rtw_wx_get_sens, /* SIOCGIWSENS */
- NULL, /* SIOCSIWRANGE */
- rtw_wx_get_range, /* SIOCGIWRANGE */
- rtw_wx_set_priv, /* SIOCSIWPRIV */
- NULL, /* SIOCGIWPRIV */
- NULL, /* SIOCSIWSTATS */
- NULL, /* SIOCGIWSTATS */
- dummy, /* SIOCSIWSPY */
- dummy, /* SIOCGIWSPY */
- NULL, /* SIOCGIWTHRSPY */
- NULL, /* SIOCWIWTHRSPY */
- rtw_wx_set_wap, /* SIOCSIWAP */
- rtw_wx_get_wap, /* SIOCGIWAP */
- rtw_wx_set_mlme, /* request MLME operation; uses struct iw_mlme */
- dummy, /* SIOCGIWAPLIST -- depricated */
- rtw_wx_set_scan, /* SIOCSIWSCAN */
- rtw_wx_get_scan, /* SIOCGIWSCAN */
- rtw_wx_set_essid, /* SIOCSIWESSID */
- rtw_wx_get_essid, /* SIOCGIWESSID */
- dummy, /* SIOCSIWNICKN */
- rtw_wx_get_nick, /* SIOCGIWNICKN */
- NULL, /* -- hole -- */
- NULL, /* -- hole -- */
- rtw_wx_set_rate, /* SIOCSIWRATE */
- rtw_wx_get_rate, /* SIOCGIWRATE */
- rtw_wx_set_rts, /* SIOCSIWRTS */
- rtw_wx_get_rts, /* SIOCGIWRTS */
- rtw_wx_set_frag, /* SIOCSIWFRAG */
- rtw_wx_get_frag, /* SIOCGIWFRAG */
- dummy, /* SIOCSIWTXPOW */
- dummy, /* SIOCGIWTXPOW */
- dummy, /* SIOCSIWRETRY */
- rtw_wx_get_retry, /* SIOCGIWRETRY */
- rtw_wx_set_enc, /* SIOCSIWENCODE */
- rtw_wx_get_enc, /* SIOCGIWENCODE */
- dummy, /* SIOCSIWPOWER */
- rtw_wx_get_power, /* SIOCGIWPOWER */
- NULL, /*---hole---*/
- NULL, /*---hole---*/
- rtw_wx_set_gen_ie, /* SIOCSIWGENIE */
- NULL, /* SIOCGWGENIE */
- rtw_wx_set_auth, /* SIOCSIWAUTH */
- NULL, /* SIOCGIWAUTH */
- rtw_wx_set_enc_ext, /* SIOCSIWENCODEEXT */
- NULL, /* SIOCGIWENCODEEXT */
- rtw_wx_set_pmkid, /* SIOCSIWPMKSA */
- NULL, /*---hole---*/
+ IW_HANDLER(SIOCGIWNAME, rtw_wx_get_name),
+ IW_HANDLER(SIOCSIWNWID, dummy),
+ IW_HANDLER(SIOCGIWNWID, dummy),
+ IW_HANDLER(SIOCSIWFREQ, rtw_wx_set_freq),
+ IW_HANDLER(SIOCGIWFREQ, rtw_wx_get_freq),
+ IW_HANDLER(SIOCSIWMODE, rtw_wx_set_mode),
+ IW_HANDLER(SIOCGIWMODE, rtw_wx_get_mode),
+ IW_HANDLER(SIOCSIWSENS, dummy),
+ IW_HANDLER(SIOCGIWSENS, rtw_wx_get_sens),
+ IW_HANDLER(SIOCGIWRANGE, rtw_wx_get_range),
+ IW_HANDLER(SIOCSIWPRIV, rtw_wx_set_priv),
+ IW_HANDLER(SIOCSIWSPY, dummy),
+ IW_HANDLER(SIOCGIWSPY, dummy),
+ IW_HANDLER(SIOCSIWAP, rtw_wx_set_wap),
+ IW_HANDLER(SIOCGIWAP, rtw_wx_get_wap),
+ IW_HANDLER(SIOCSIWMLME, rtw_wx_set_mlme),
+ IW_HANDLER(SIOCGIWAPLIST, dummy),
+ IW_HANDLER(SIOCSIWSCAN, rtw_wx_set_scan),
+ IW_HANDLER(SIOCGIWSCAN, rtw_wx_get_scan),
+ IW_HANDLER(SIOCSIWESSID, rtw_wx_set_essid),
+ IW_HANDLER(SIOCGIWESSID, rtw_wx_get_essid),
+ IW_HANDLER(SIOCSIWNICKN, dummy),
+ IW_HANDLER(SIOCGIWNICKN, rtw_wx_get_nick),
+ IW_HANDLER(SIOCSIWRATE, rtw_wx_set_rate),
+ IW_HANDLER(SIOCGIWRATE, rtw_wx_get_rate),
+ IW_HANDLER(SIOCSIWRTS, rtw_wx_set_rts),
+ IW_HANDLER(SIOCGIWRTS, rtw_wx_get_rts),
+ IW_HANDLER(SIOCSIWFRAG, rtw_wx_set_frag),
+ IW_HANDLER(SIOCGIWFRAG, rtw_wx_get_frag),
+ IW_HANDLER(SIOCSIWTXPOW, dummy),
+ IW_HANDLER(SIOCGIWTXPOW, dummy),
+ IW_HANDLER(SIOCSIWRETRY, dummy),
+ IW_HANDLER(SIOCGIWRETRY, rtw_wx_get_retry),
+ IW_HANDLER(SIOCSIWENCODE, rtw_wx_set_enc),
+ IW_HANDLER(SIOCGIWENCODE, rtw_wx_get_enc),
+ IW_HANDLER(SIOCSIWPOWER, dummy),
+ IW_HANDLER(SIOCGIWPOWER, rtw_wx_get_power),
+ IW_HANDLER(SIOCSIWGENIE, rtw_wx_set_gen_ie),
+ IW_HANDLER(SIOCSIWAUTH, rtw_wx_set_auth),
+ IW_HANDLER(SIOCSIWENCODEEXT, rtw_wx_set_enc_ext),
+ IW_HANDLER(SIOCSIWPMKSA, rtw_wx_set_pmkid),
};
static struct iw_statistics *rtw_get_wireless_stats(struct net_device *dev)
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] staging: rtl8188eu: remove dummy wext handlers
2021-05-29 12:13 [PATCH 1/3] staging: rtl8188eu: use IW_HANDLER to declare wext handlers Martin Kaiser
@ 2021-05-29 12:13 ` Martin Kaiser
2021-05-31 6:02 ` Dan Carpenter
2021-05-29 12:13 ` [PATCH 3/3] staging: rtl8188eu: remove dummy setfreq handler Martin Kaiser
1 sibling, 1 reply; 5+ messages in thread
From: Martin Kaiser @ 2021-05-29 12:13 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman
Cc: linux-staging, kernel-janitors, linux-kernel, Martin Kaiser
Remove the wext handlers that link to an empty dummy function.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
If no handler is installed for a wext ioctl, the wext core will return
-ENOTSUPP to user space. The dummy function returned -1. It could be argued
that this change breaks the user space ABI.
However, it's rather unlikely that an application expects a particular wlan
driver and chipset when it configures a wireless network. Checking for
errno==1 explicitly will already be non-portable as most other drivers set
errn==ENOTSUPP for non-existing ioctls.
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index a23ccda6fefe..78699999c4fe 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -1910,12 +1910,6 @@ static int rtw_wx_get_nick(struct net_device *dev,
return 0;
}
-static int dummy(struct net_device *dev, struct iw_request_info *a,
- union iwreq_data *wrqu, char *b)
-{
- return -1;
-}
-
static int wpa_set_param(struct net_device *dev, u8 name, u32 value)
{
uint ret = 0;
@@ -2900,27 +2894,20 @@ static int rtw_wx_set_priv(struct net_device *dev,
static iw_handler rtw_handlers[] = {
IW_HANDLER(SIOCGIWNAME, rtw_wx_get_name),
- IW_HANDLER(SIOCSIWNWID, dummy),
- IW_HANDLER(SIOCGIWNWID, dummy),
IW_HANDLER(SIOCSIWFREQ, rtw_wx_set_freq),
IW_HANDLER(SIOCGIWFREQ, rtw_wx_get_freq),
IW_HANDLER(SIOCSIWMODE, rtw_wx_set_mode),
IW_HANDLER(SIOCGIWMODE, rtw_wx_get_mode),
- IW_HANDLER(SIOCSIWSENS, dummy),
IW_HANDLER(SIOCGIWSENS, rtw_wx_get_sens),
IW_HANDLER(SIOCGIWRANGE, rtw_wx_get_range),
IW_HANDLER(SIOCSIWPRIV, rtw_wx_set_priv),
- IW_HANDLER(SIOCSIWSPY, dummy),
- IW_HANDLER(SIOCGIWSPY, dummy),
IW_HANDLER(SIOCSIWAP, rtw_wx_set_wap),
IW_HANDLER(SIOCGIWAP, rtw_wx_get_wap),
IW_HANDLER(SIOCSIWMLME, rtw_wx_set_mlme),
- IW_HANDLER(SIOCGIWAPLIST, dummy),
IW_HANDLER(SIOCSIWSCAN, rtw_wx_set_scan),
IW_HANDLER(SIOCGIWSCAN, rtw_wx_get_scan),
IW_HANDLER(SIOCSIWESSID, rtw_wx_set_essid),
IW_HANDLER(SIOCGIWESSID, rtw_wx_get_essid),
- IW_HANDLER(SIOCSIWNICKN, dummy),
IW_HANDLER(SIOCGIWNICKN, rtw_wx_get_nick),
IW_HANDLER(SIOCSIWRATE, rtw_wx_set_rate),
IW_HANDLER(SIOCGIWRATE, rtw_wx_get_rate),
@@ -2928,13 +2915,9 @@ static iw_handler rtw_handlers[] = {
IW_HANDLER(SIOCGIWRTS, rtw_wx_get_rts),
IW_HANDLER(SIOCSIWFRAG, rtw_wx_set_frag),
IW_HANDLER(SIOCGIWFRAG, rtw_wx_get_frag),
- IW_HANDLER(SIOCSIWTXPOW, dummy),
- IW_HANDLER(SIOCGIWTXPOW, dummy),
- IW_HANDLER(SIOCSIWRETRY, dummy),
IW_HANDLER(SIOCGIWRETRY, rtw_wx_get_retry),
IW_HANDLER(SIOCSIWENCODE, rtw_wx_set_enc),
IW_HANDLER(SIOCGIWENCODE, rtw_wx_get_enc),
- IW_HANDLER(SIOCSIWPOWER, dummy),
IW_HANDLER(SIOCGIWPOWER, rtw_wx_get_power),
IW_HANDLER(SIOCSIWGENIE, rtw_wx_set_gen_ie),
IW_HANDLER(SIOCSIWAUTH, rtw_wx_set_auth),
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/3] staging: rtl8188eu: remove dummy wext handlers
2021-05-29 12:13 ` [PATCH 2/3] staging: rtl8188eu: remove dummy " Martin Kaiser
@ 2021-05-31 6:02 ` Dan Carpenter
2021-05-31 7:55 ` Martin Kaiser
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2021-05-31 6:02 UTC (permalink / raw)
To: Martin Kaiser
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, kernel-janitors,
linux-kernel
On Sat, May 29, 2021 at 02:13:45PM +0200, Martin Kaiser wrote:
> Remove the wext handlers that link to an empty dummy function.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> If no handler is installed for a wext ioctl, the wext core will return
> -ENOTSUPP to user space. The dummy function returned -1. It could be argued
> that this change breaks the user space ABI.
>
> However, it's rather unlikely that an application expects a particular wlan
> driver and chipset when it configures a wireless network. Checking for
> errno==1 explicitly will already be non-portable as most other drivers set
> errn==ENOTSUPP for non-existing ioctls.
>
The patch is good, but next time, just put all this commentary in the
commit message. It has to do with the impact of the patch on userspace
and it's helpful for the reviewers. Some reviewers won't need it, but
I try to target my commit messages at developers who aren't subsystem
experts. I also like that it shows you thought about it.
I recently saw someone claim that "anyone who can review these patches
will already know what the <SOMETHING> acronym stands for so that's why
I didn't explain." And I was like, "Oh wow... That's a harsh burn on
me!"
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] staging: rtl8188eu: remove dummy wext handlers
2021-05-31 6:02 ` Dan Carpenter
@ 2021-05-31 7:55 ` Martin Kaiser
0 siblings, 0 replies; 5+ messages in thread
From: Martin Kaiser @ 2021-05-31 7:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, kernel-janitors,
linux-kernel
Hi Dan and all,
Thus wrote Dan Carpenter (dan.carpenter@oracle.com):
> The patch is good, but next time, just put all this commentary in the
> commit message.
I was asking myself if this "expression of my doubts" should go into the
commit message or not.
Thanks for the feedback.
Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] staging: rtl8188eu: remove dummy setfreq handler
2021-05-29 12:13 [PATCH 1/3] staging: rtl8188eu: use IW_HANDLER to declare wext handlers Martin Kaiser
2021-05-29 12:13 ` [PATCH 2/3] staging: rtl8188eu: remove dummy " Martin Kaiser
@ 2021-05-29 12:13 ` Martin Kaiser
1 sibling, 0 replies; 5+ messages in thread
From: Martin Kaiser @ 2021-05-29 12:13 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman
Cc: linux-staging, kernel-janitors, linux-kernel, Martin Kaiser
The setfreq handler contains only a debug print. It returns success
although the frequency wasn't set.
Remove this handler and inform callers that this driver does not
support setfreq.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index 78699999c4fe..ebd61ba93ef0 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -667,14 +667,6 @@ static int rtw_wx_get_name(struct net_device *dev,
return 0;
}
-static int rtw_wx_set_freq(struct net_device *dev,
- struct iw_request_info *info,
- union iwreq_data *wrqu, char *extra)
-{
- RT_TRACE(_module_rtl871x_mlme_c_, _drv_notice_, ("+%s\n", __func__));
- return 0;
-}
-
static int rtw_wx_get_freq(struct net_device *dev,
struct iw_request_info *info,
union iwreq_data *wrqu, char *extra)
@@ -2894,7 +2886,6 @@ static int rtw_wx_set_priv(struct net_device *dev,
static iw_handler rtw_handlers[] = {
IW_HANDLER(SIOCGIWNAME, rtw_wx_get_name),
- IW_HANDLER(SIOCSIWFREQ, rtw_wx_set_freq),
IW_HANDLER(SIOCGIWFREQ, rtw_wx_get_freq),
IW_HANDLER(SIOCSIWMODE, rtw_wx_set_mode),
IW_HANDLER(SIOCGIWMODE, rtw_wx_get_mode),
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-31 7:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-29 12:13 [PATCH 1/3] staging: rtl8188eu: use IW_HANDLER to declare wext handlers Martin Kaiser
2021-05-29 12:13 ` [PATCH 2/3] staging: rtl8188eu: remove dummy " Martin Kaiser
2021-05-31 6:02 ` Dan Carpenter
2021-05-31 7:55 ` Martin Kaiser
2021-05-29 12:13 ` [PATCH 3/3] staging: rtl8188eu: remove dummy setfreq handler Martin Kaiser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).