<note, your Reply-To: is very odd, please fix your email client up...>
On Tue, Apr 18, 2023 at 02:35:52PM +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.
>
> Reported by checkpatch:
>
> CHECK: Macro argument reuse 'N' - possible side-effects?
>
> Signed off by: Luke Koch <lu.ale.koch@xxxxxxxxx>
> ---
> v2: - Fix array underflow and conditions with respect to the start at 0
> v3: - Remove unnecessary spaces
> ---
> 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;
> - 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..fc465261baa1 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++) {
> + if (count > i &&
> + DOT11_RATE5_ISBASIC_GET(item->supprates[i])) {
> + req->basicrate[i].data = item->supprates[i];
> + 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];
> + req->supprate[i].status =
> + P80211ENUM_msgitem_status_data_ok;
> + }
> + }
This patch implies that these structures are set but never actually read
from, so why are they present at all? Is this a structure that is on
the wire/air or used somewhere else as an api to hardware?
I tried to unwind things in the driver, but couldn't figure it out, what
happens if you just delete these fields, does the driver still work
properly?
thanks,
greg k-h
