2022-02-20 17:55:04

by Michael Straube

Subject: [PATCH 2/3] staging: r8188eu: refactor rtw_ch2freq()

Since we have only channel values from 1 to 14 we can convert the
array ch_freq_map to a simple integer array and use the indices as
channel numbers. This simplifies the code and avoids looping through
the array to get the frequency.

Signed-off-by: Michael Straube <[email protected]>
---
drivers/staging/r8188eu/core/rtw_rf.c | 40 +++++++++++----------------
1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_rf.c b/drivers/staging/r8188eu/core/rtw_rf.c
index 51425971782b..945a40e6511a 100644
--- a/drivers/staging/r8188eu/core/rtw_rf.c
+++ b/drivers/staging/r8188eu/core/rtw_rf.c
@@ -8,32 +8,24 @@
#include "../include/recv_osdep.h"
#include "../include/xmit_osdep.h"

-struct ch_freq {
- u32 channel;
- u32 frequency;
+static const u32 ch_freq_map[] = {
+ 2412,
+ 2417,
+ 2422,
+ 2427,
+ 2432,
+ 2437,
+ 2442,
+ 2447,
+ 2452,
+ 2457,
+ 2462,
+ 2467,
+ 2472,
+ 2484
};

-static struct ch_freq ch_freq_map[] = {
- {1, 2412}, {2, 2417}, {3, 2422}, {4, 2427}, {5, 2432},
- {6, 2437}, {7, 2442}, {8, 2447}, {9, 2452}, {10, 2457},
- {11, 2462}, {12, 2467}, {13, 2472}, {14, 2484},
-};
-
-static int ch_freq_map_num = ARRAY_SIZE(ch_freq_map);
-
u32 rtw_ch2freq(u32 channel)
{
- u8 i;
- u32 freq = 0;
-
- for (i = 0; i < ch_freq_map_num; i++) {
- if (channel == ch_freq_map[i].channel) {
- freq = ch_freq_map[i].frequency;
- break;
- }
- }
- if (i == ch_freq_map_num)
- freq = 2412;
-
- return freq;
+ return ch_freq_map[channel - 1];
}
--
2.35.1

2022-02-21 09:52:14

by Pavel Skripkin

Subject: Re: [PATCH 2/3] staging: r8188eu: refactor rtw_ch2freq()

Hi Michael,

On 2/20/22 18:48, Michael Straube wrote:
> -static int ch_freq_map_num = ARRAY_SIZE(ch_freq_map);
> -
> u32 rtw_ch2freq(u32 channel)
> {
> - u8 i;
> - u32 freq = 0;
> -
> - for (i = 0; i < ch_freq_map_num; i++) {
> - if (channel == ch_freq_map[i].channel) {
> - freq = ch_freq_map[i].frequency;
> - break;
> - }
> - }
> - if (i == ch_freq_map_num)
> - freq = 2412;
> -
> - return freq;
> + return ch_freq_map[channel - 1];
> }

What if channel has wrong value? The old code returned some default
value, but with new one we will hit OOB.





With regards,
Pavel Skripkin