On Mon, Jan 18, 2021 at 02:09:56AM +0100, Johannes Czekay wrote:
> This patch cleans up two "macro argument reuse" warnings by checkpatch.
> This should also make the code much more readable.
> 
> Signed-off-by: Johannes Czekay <johannes.cze...@fau.de>
> Co-developed-by: Nicolai Fischer <nicolai.fisc...@fau.de>
> Signed-off-by: Nicolai Fischer <nicolai.fisc...@fau.de>
> ---
>  drivers/staging/wlan-ng/p80211metastruct.h | 18 +-------
>  drivers/staging/wlan-ng/prism2mgmt.c       | 48 ++++++----------------
>  2 files changed, 14 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 1bd36dc2b7ff..8540c3336907 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.c
> +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> @@ -388,6 +388,7 @@ int prism2mgmt_scan_results(struct wlandevice *wlandev, 
> void *msgp)
>       struct hfa384x_hscan_result_sub *item = NULL;
>  
>       int count;
> +     int i;
>  
>       req = msgp;
>  
> @@ -437,42 +438,17 @@ 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 (i = 0; i < 8; i++) {
> +             if (count > 1) {

This should be "i" instead of "1".  And conditions are more readable if
you put the part that changes first "if (variable < limit)" to avoid
backwards think.

        if (i < count) {

Or you could reverse it the other way:

        if (i >= count)
                break;

> +                     if (DOT11_RATE5_ISBASIC_GET(item->supprates[i])) {
> +                             req->basicrate[i].data = item->supprates[i];
> +                             req->basicrate[i].status = 
> P80211ENUM_msgitem_status_data_ok;
> +                     }
> +
> +                     req->supprate[i].data = item->supprates[i];
> +                     req->supprate[i].status = 
> P80211ENUM_msgitem_status_data_ok;
> +             }
> +     }

It's sort of surprising that we can change how we write this information
but we don't have to change how it is read.  I guess presumably it's
just dumped as hex to debugfs or something like that...  Who knows.  :/

regards,
dan carpenter

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to