* [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
@ 2021-07-01 14:47 Fabio M. De Francesco
2021-07-02 7:48 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Fabio M. De Francesco @ 2021-07-01 14:47 UTC (permalink / raw)
To: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
Cc: Fabio M. De Francesco
Remove set but unused iw_operation_mode[]. Remove all the lines of
code from the function rtw_wx_set_rate, except the "return 0;" line
to not break userland code that somewhat uses this IOCTL.
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
.../staging/rtl8188eu/os_dep/ioctl_linux.c | 72 -------------------
1 file changed, 72 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index b958a8d882b0..a66d8838c034 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -46,11 +46,6 @@ static u32 rtw_rates[] = {1000000, 2000000, 5500000, 11000000,
6000000, 9000000, 12000000, 18000000, 24000000, 36000000,
48000000, 54000000};
-static const char * const iw_operation_mode[] = {
- "Auto", "Ad-Hoc", "Managed", "Master", "Repeater",
- "Secondary", "Monitor"
-};
-
void indicate_wx_scan_complete_event(struct adapter *padapter)
{
union iwreq_data wrqu;
@@ -1266,73 +1261,6 @@ static int rtw_wx_set_rate(struct net_device *dev,
struct iw_request_info *a,
union iwreq_data *wrqu, char *extra)
{
- int i;
- u8 datarates[NumRates];
- u32 target_rate = wrqu->bitrate.value;
- u32 fixed = wrqu->bitrate.fixed;
- u32 ratevalue = 0;
- u8 mpdatarate[NumRates] = {11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 0xff};
-
- if (target_rate == -1) {
- ratevalue = 11;
- goto set_rate;
- }
- target_rate /= 100000;
-
- switch (target_rate) {
- case 10:
- ratevalue = 0;
- break;
- case 20:
- ratevalue = 1;
- break;
- case 55:
- ratevalue = 2;
- break;
- case 60:
- ratevalue = 3;
- break;
- case 90:
- ratevalue = 4;
- break;
- case 110:
- ratevalue = 5;
- break;
- case 120:
- ratevalue = 6;
- break;
- case 180:
- ratevalue = 7;
- break;
- case 240:
- ratevalue = 8;
- break;
- case 360:
- ratevalue = 9;
- break;
- case 480:
- ratevalue = 10;
- break;
- case 540:
- ratevalue = 11;
- break;
- default:
- ratevalue = 11;
- break;
- }
-
-set_rate:
-
- for (i = 0; i < NumRates; i++) {
- if (ratevalue == mpdatarate[i]) {
- datarates[i] = mpdatarate[i];
- if (fixed == 0)
- break;
- } else {
- datarates[i] = 0xff;
- }
- }
-
return 0;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
2021-07-01 14:47 [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code Fabio M. De Francesco
@ 2021-07-02 7:48 ` Dan Carpenter
2021-07-02 8:35 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-07-02 7:48 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
On Thu, Jul 01, 2021 at 04:47:07PM +0200, Fabio M. De Francesco wrote:
> Remove set but unused iw_operation_mode[]. Remove all the lines of
> code from the function rtw_wx_set_rate, except the "return 0;" line
> to not break userland code that somewhat uses this IOCTL.
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> .../staging/rtl8188eu/os_dep/ioctl_linux.c | 72 -------------------
> 1 file changed, 72 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index b958a8d882b0..a66d8838c034 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -46,11 +46,6 @@ static u32 rtw_rates[] = {1000000, 2000000, 5500000, 11000000,
> 6000000, 9000000, 12000000, 18000000, 24000000, 36000000,
> 48000000, 54000000};
>
> -static const char * const iw_operation_mode[] = {
> - "Auto", "Ad-Hoc", "Managed", "Master", "Repeater",
> - "Secondary", "Monitor"
> -};
> -
> void indicate_wx_scan_complete_event(struct adapter *padapter)
> {
> union iwreq_data wrqu;
> @@ -1266,73 +1261,6 @@ static int rtw_wx_set_rate(struct net_device *dev,
> struct iw_request_info *a,
> union iwreq_data *wrqu, char *extra)
> {
Just delete this whole file. It doesn't do anything now.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
2021-07-02 7:48 ` Dan Carpenter
@ 2021-07-02 8:35 ` Dan Carpenter
2021-07-02 12:14 ` Fabio M. De Francesco
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-07-02 8:35 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
On Fri, Jul 02, 2021 at 10:48:40AM +0300, Dan Carpenter wrote:
> On Thu, Jul 01, 2021 at 04:47:07PM +0200, Fabio M. De Francesco wrote:
> > Remove set but unused iw_operation_mode[]. Remove all the lines of
> > code from the function rtw_wx_set_rate, except the "return 0;" line
> > to not break userland code that somewhat uses this IOCTL.
> >
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > .../staging/rtl8188eu/os_dep/ioctl_linux.c | 72 -------------------
> > 1 file changed, 72 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > index b958a8d882b0..a66d8838c034 100644
> > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > @@ -46,11 +46,6 @@ static u32 rtw_rates[] = {1000000, 2000000, 5500000, 11000000,
> > 6000000, 9000000, 12000000, 18000000, 24000000, 36000000,
> > 48000000, 54000000};
> >
> > -static const char * const iw_operation_mode[] = {
> > - "Auto", "Ad-Hoc", "Managed", "Master", "Repeater",
> > - "Secondary", "Monitor"
> > -};
> > -
> > void indicate_wx_scan_complete_event(struct adapter *padapter)
> > {
> > union iwreq_data wrqu;
> > @@ -1266,73 +1261,6 @@ static int rtw_wx_set_rate(struct net_device *dev,
> > struct iw_request_info *a,
> > union iwreq_data *wrqu, char *extra)
> > {
>
> Just delete this whole file. It doesn't do anything now.
Sorry, I meant function, not file. *chortle*. :P
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
2021-07-02 8:35 ` Dan Carpenter
@ 2021-07-02 12:14 ` Fabio M. De Francesco
2021-07-02 12:42 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Fabio M. De Francesco @ 2021-07-02 12:14 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
On Friday, July 2, 2021 10:35:21 AM CEST Dan Carpenter wrote:
> On Fri, Jul 02, 2021 at 10:48:40AM +0300, Dan Carpenter wrote:
> > On Thu, Jul 01, 2021 at 04:47:07PM +0200, Fabio M. De Francesco wrote:
> > > Remove set but unused iw_operation_mode[]. Remove all the lines of
> > > code from the function rtw_wx_set_rate, except the "return 0;" line
> > > to not break userland code that somewhat uses this IOCTL.
> > >
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > [...]
> >
Dear Dan,
> > Just delete this whole file. It doesn't do anything now.
>
> Sorry, I meant function, not file. *chortle*. :P
No worries, it is clear it was unintended.
Back to the function... As you may suspect :-) I know practically nothing
neither of Linux device drivers or of whatever else kernel, so I take your
words for good. ASAP, I'll send a v2 of this patch.
However, I usually like to understand what I make (just for fun and... more).
That rtw_wx_set_rate() is the implementation of the SIOCSIWRATE IOCTL command.
I hope that I have not misunderstood it, have I?
However, we know that this function does practically nothing and then simply
returns 0 to the user. That's exactly the reason why I deleted all its lines
(except one).
If I am a user of that command I get a "success" return code (0) and I don't
notice that it won't be able to set the bit rate. However everything should
still keep running (I suppose using the default bit rate of the hardware; who
really knows?).
Now it's time for two questions:
1) what happens if that command is used by some users that (hopelessly) expect
the function to set the bit rate? I mean: if the function is not anymore in
the list of the IOCTL commands of the rtw_handlers array will still the user
program compile, link, and don't crash at runtime?
2) how should I delete the association of SIOCSIWRATE with rtw_wx_set_rate()
in the rtw_handlers array?
- delete the entry and shift the array one position up?
- set the SIOCSIWRATE entry to NULL?
Regards,
Fabio
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
2021-07-02 12:14 ` Fabio M. De Francesco
@ 2021-07-02 12:42 ` Dan Carpenter
2021-07-02 12:46 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-07-02 12:42 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
On Fri, Jul 02, 2021 at 02:14:45PM +0200, Fabio M. De Francesco wrote:
> On Friday, July 2, 2021 10:35:21 AM CEST Dan Carpenter wrote:
> > On Fri, Jul 02, 2021 at 10:48:40AM +0300, Dan Carpenter wrote:
> > > On Thu, Jul 01, 2021 at 04:47:07PM +0200, Fabio M. De Francesco wrote:
> > > > Remove set but unused iw_operation_mode[]. Remove all the lines of
> > > > code from the function rtw_wx_set_rate, except the "return 0;" line
> > > > to not break userland code that somewhat uses this IOCTL.
> > > >
> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > [...]
> > >
>
> Dear Dan,
>
> > > Just delete this whole file. It doesn't do anything now.
> >
> > Sorry, I meant function, not file. *chortle*. :P
>
> No worries, it is clear it was unintended.
>
> Back to the function... As you may suspect :-) I know practically nothing
> neither of Linux device drivers or of whatever else kernel, so I take your
> words for good. ASAP, I'll send a v2 of this patch.
>
> However, I usually like to understand what I make (just for fun and... more).
>
> That rtw_wx_set_rate() is the implementation of the SIOCSIWRATE IOCTL command.
> I hope that I have not misunderstood it, have I?
Correct.
>
> However, we know that this function does practically nothing and then simply
> returns 0 to the user. That's exactly the reason why I deleted all its lines
> (except one).
It used to do nothing in a much more complicated way before commit
1aef69ecacda ("staging: rtl8188eu: Remove function rtw_setdatarate_cmd()")
>
> If I am a user of that command I get a "success" return code (0) and I don't
> notice that it won't be able to set the bit rate. However everything should
> still keep running (I suppose using the default bit rate of the hardware; who
> really knows?).
It will still do nothing but now instead of returning success it will
return -ENOTSUPP. This is done in wireless_process_ioctl().
>
> Now it's time for two questions:
>
> 1) what happens if that command is used by some users that (hopelessly) expect
> the function to set the bit rate? I mean: if the function is not anymore in
> the list of the IOCTL commands of the rtw_handlers array will still the user
> program compile, link, and don't crash at runtime?
>
Userspace programs are supposed to be written so that they work with
every wifi driver, so they should be able to handle -ENOTSUPP. If this
breaks a userspace application then we will have to change it back. But
returning -ENOTSUPP is the correct behavior, so let's first try to do
the correct thing and then think about working around bugs in userspace
if we find them.
> 2) how should I delete the association of SIOCSIWRATE with rtw_wx_set_rate()
> in the rtw_handlers array?
> - delete the entry and shift the array one position up?
> - set the SIOCSIWRATE entry to NULL?
The IW_HANDLER() macro puts the function in the correct position in the
array. So just delete it. Everything else will remain unchanged.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code
2021-07-02 12:42 ` Dan Carpenter
@ 2021-07-02 12:46 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2021-07-02 12:46 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Larry Finger, Greg Kroah-Hartman, linux-staging, linux-kernel
You should write that at the start of the commit message.
This driver doesn't support SIOCSIWRATE. It just returns zero and does
nothing. Change it to return -ENOTSUPP instead. (This is an API change
but we don't expect it to break anything).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-02 12:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 14:47 [PATCH] staging: rtl8188eu: Remove an unused variable and some lines of code Fabio M. De Francesco
2021-07-02 7:48 ` Dan Carpenter
2021-07-02 8:35 ` Dan Carpenter
2021-07-02 12:14 ` Fabio M. De Francesco
2021-07-02 12:42 ` Dan Carpenter
2021-07-02 12:46 ` Dan Carpenter
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).