On Sat, Apr 15, 2023 at 01:28:47PM +0200, Luke Koch wrote:
> Change p80211msg_dot11req_scan_results rate members to struct arrays
> instead of individually numbered member structs.
> Replace macros to set rates with loops to avoid checkpatch warning
> and adhere to linux coding style.
Writing every sentence in imperative tense makes people sound like space
aliens in human skin... You can if you want to but it's not a
requirement in staging. There are two maintainers who have that
requirement and the Outreachy people.
>
> Reported by checkpatch:
>
> CHECK: Macro argument reuse 'N' - possible side-effects?
>
> Signed off by: Luke Koch <lu.ale.koch@xxxxxxxxx>
> ---
> drivers/staging/wlan-ng/p80211metastruct.h | 18 +-------
> drivers/staging/wlan-ng/prism2mgmt.c | 52 +++++++---------------
> 2 files changed, 18 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/staging/wlan-ng/p80211metastruct.h b/drivers/staging/wlan-ng/p80211metastruct.h
> index 4adc64580185..e963227f797c 100644
> --- a/drivers/staging/wlan-ng/p80211metastruct.h
> +++ b/drivers/staging/wlan-ng/p80211metastruct.h
> @@ -114,22 +114,8 @@ struct p80211msg_dot11req_scan_results {
> struct p80211item_uint32 cfpollreq;
> struct p80211item_uint32 privacy;
> struct p80211item_uint32 capinfo;
> - struct p80211item_uint32 basicrate1;
^^^^^^^^^^
This starts at 1 instead of 0.
> - struct p80211item_uint32 basicrate2;
> - struct p80211item_uint32 basicrate3;
> - struct p80211item_uint32 basicrate4;
> - struct p80211item_uint32 basicrate5;
> - struct p80211item_uint32 basicrate6;
> - struct p80211item_uint32 basicrate7;
> - struct p80211item_uint32 basicrate8;
> - struct p80211item_uint32 supprate1;
> - struct p80211item_uint32 supprate2;
> - struct p80211item_uint32 supprate3;
> - struct p80211item_uint32 supprate4;
> - struct p80211item_uint32 supprate5;
> - struct p80211item_uint32 supprate6;
> - struct p80211item_uint32 supprate7;
> - struct p80211item_uint32 supprate8;
> + struct p80211item_uint32 basicrate[8];
> + struct p80211item_uint32 supprate[8];
> } __packed;
>
> struct p80211msg_dot11req_start {
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.c b/drivers/staging/wlan-ng/prism2mgmt.c
> index 9030a8939a9b..069c39c36753 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -437,42 +437,22 @@ int prism2mgmt_scan_results(struct wlandevice *wlandev, void *msgp)
> if (item->supprates[count] == 0)
> break;
>
> -#define REQBASICRATE(N) \
> - do { \
> - if ((count >= (N)) && DOT11_RATE5_ISBASIC_GET( \
> - item->supprates[(N) - 1])) { \
> - req->basicrate ## N .data = item->supprates[(N) - 1]; \
> - req->basicrate ## N .status = \
> - P80211ENUM_msgitem_status_data_ok; \
> - } \
> - } while (0)
> -
> - REQBASICRATE(1);
> - REQBASICRATE(2);
> - REQBASICRATE(3);
> - REQBASICRATE(4);
> - REQBASICRATE(5);
> - REQBASICRATE(6);
> - REQBASICRATE(7);
> - REQBASICRATE(8);
> -
> -#define REQSUPPRATE(N) \
> - do { \
> - if (count >= (N)) { \
> - req->supprate ## N .data = item->supprates[(N) - 1]; \
> - req->supprate ## N .status = \
> - P80211ENUM_msgitem_status_data_ok; \
> - } \
> - } while (0)
> -
> - REQSUPPRATE(1);
> - REQSUPPRATE(2);
> - REQSUPPRATE(3);
> - REQSUPPRATE(4);
> - REQSUPPRATE(5);
> - REQSUPPRATE(6);
> - REQSUPPRATE(7);
> - REQSUPPRATE(8);
> + for (int i = 0; i < 8; i++) {
^^^^^
Now it starts at zero.
> + if (count >= i &&
> + DOT11_RATE5_ISBASIC_GET(item->supprates[i - 1])) {
^^^^^
This is an array underflow.
regards,
dan carpenter
> + req->basicrate[i] .data = item->supprates[i - 1];
> + req->basicrate[i] .status =
> + P80211ENUM_msgitem_status_data_ok;
> + }
> + }
> +
> + for (int i = 0; i < 8; i++) {
> + if (count >= i) {
> + req->supprate[i] .data = item->supprates[i - 1];
> + req->supprate[i] .status =
> + P80211ENUM_msgitem_status_data_ok;
> + }
> + }
>
> /* beacon period */
> req->beaconperiod.status = P80211ENUM_msgitem_status_data_ok;
> --
> 2.34.1
>
