* [PATCH v1] ath5k: fix WARNING opportunity for swap.
@ 2023-08-14 20:02 Mahmoud Maatuq
2023-08-14 22:28 ` Jeff Johnson
0 siblings, 1 reply; 3+ messages in thread
From: Mahmoud Maatuq @ 2023-08-14 20:02 UTC (permalink / raw)
To: jirislaby, mickflemm, mcgrof, kvalo, linux-wireless, linux-kernel
Cc: linux-kernel-mentees, Mahmoud Maatuq
coccinielle reported the following:
./drivers/net/wireless/ath/ath5k/phy.c:1573:25-26: WARNING opportunity for swap()
Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
---
changes in v1:
- replace the entire double loop with sort()
as suggested by Jiri Slaby <jirislaby@kernel.org>
---
drivers/net/wireless/ath/ath5k/phy.c | 29 +++++++++++++---------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 5797ef9c73d7..7ee4e1616f45 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -26,6 +26,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/sort.h>
#include <asm/unaligned.h>
#include "ath5k.h"
@@ -1554,6 +1555,11 @@ static void ath5k_hw_update_nfcal_hist(struct ath5k_hw *ah, s16 noise_floor)
hist->nfval[hist->index] = noise_floor;
}
+static int cmps16(const void *a, const void *b)
+{
+ return *(s16 *)a - *(s16 *)b;
+}
+
/**
* ath5k_hw_get_median_noise_floor() - Get median NF from history buffer
* @ah: The &struct ath5k_hw
@@ -1561,25 +1567,16 @@ static void ath5k_hw_update_nfcal_hist(struct ath5k_hw *ah, s16 noise_floor)
static s16
ath5k_hw_get_median_noise_floor(struct ath5k_hw *ah)
{
- s16 sort[ATH5K_NF_CAL_HIST_MAX];
- s16 tmp;
- int i, j;
-
- memcpy(sort, ah->ah_nfcal_hist.nfval, sizeof(sort));
- for (i = 0; i < ATH5K_NF_CAL_HIST_MAX - 1; i++) {
- for (j = 1; j < ATH5K_NF_CAL_HIST_MAX - i; j++) {
- if (sort[j] > sort[j - 1]) {
- tmp = sort[j];
- sort[j] = sort[j - 1];
- sort[j - 1] = tmp;
- }
- }
- }
+ s16 sorted_nfval[ATH5K_NF_CAL_HIST_MAX];
+ int i;
+
+ memcpy(sorted_nfval, ah->ah_nfcal_hist.nfval, sizeof(sorted_nfval));
+ sort(sorted_nfval, ATH5K_NF_CAL_HIST_MAX, sizeof(s16), cmps16, NULL);
for (i = 0; i < ATH5K_NF_CAL_HIST_MAX; i++) {
ATH5K_DBG(ah, ATH5K_DEBUG_CALIBRATE,
- "cal %d:%d\n", i, sort[i]);
+ "cal %d:%d\n", i, sorted_nfval[i]);
}
- return sort[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
+ return sorted_nfval[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v1] ath5k: fix WARNING opportunity for swap.
2023-08-14 20:02 [PATCH v1] ath5k: fix WARNING opportunity for swap Mahmoud Maatuq
@ 2023-08-14 22:28 ` Jeff Johnson
2023-08-14 22:31 ` Jeff Johnson
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Johnson @ 2023-08-14 22:28 UTC (permalink / raw)
To: Mahmoud Maatuq, jirislaby, mickflemm, mcgrof, kvalo,
linux-wireless, linux-kernel
Cc: linux-kernel-mentees
On 8/14/2023 1:02 PM, Mahmoud Maatuq wrote:
> coccinielle reported the following:
> ./drivers/net/wireless/ath/ath5k/phy.c:1573:25-26: WARNING opportunity for swap()
Suggest you add something like:
This revealed that ath5k_hw_get_median_noise_floor() had open-coded
sort() functionality. Since ath5k_hw_get_median_noise_floor() only
executes once every 10 seconds, any extra overhead due to sort() calling
it's "compare" and "swap" functions can be ignored, so replace the
existing logic with a call to sort().
and before your SOB add:
Suggested-by: Jiri Slaby <jirislaby@kernel.org>
> Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
> ---
> changes in v1:
> - replace the entire double loop with sort()
> as suggested by Jiri Slaby <jirislaby@kernel.org>
> ---
> drivers/net/wireless/ath/ath5k/phy.c | 29 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
> index 5797ef9c73d7..7ee4e1616f45 100644
> --- a/drivers/net/wireless/ath/ath5k/phy.c
> +++ b/drivers/net/wireless/ath/ath5k/phy.c
> @@ -26,6 +26,7 @@
>
> #include <linux/delay.h>
> #include <linux/slab.h>
> +#include <linux/sort.h>
> #include <asm/unaligned.h>
>
> #include "ath5k.h"
> @@ -1554,6 +1555,11 @@ static void ath5k_hw_update_nfcal_hist(struct ath5k_hw *ah, s16 noise_floor)
> hist->nfval[hist->index] = noise_floor;
> }
>
> +static int cmps16(const void *a, const void *b)
> +{
> + return *(s16 *)a - *(s16 *)b;
> +}
> +
> /**
> * ath5k_hw_get_median_noise_floor() - Get median NF from history buffer
> * @ah: The &struct ath5k_hw
> @@ -1561,25 +1567,16 @@ static void ath5k_hw_update_nfcal_hist(struct ath5k_hw *ah, s16 noise_floor)
> static s16
> ath5k_hw_get_median_noise_floor(struct ath5k_hw *ah)
> {
> - s16 sort[ATH5K_NF_CAL_HIST_MAX];
> - s16 tmp;
> - int i, j;
> -
> - memcpy(sort, ah->ah_nfcal_hist.nfval, sizeof(sort));
> - for (i = 0; i < ATH5K_NF_CAL_HIST_MAX - 1; i++) {
> - for (j = 1; j < ATH5K_NF_CAL_HIST_MAX - i; j++) {
> - if (sort[j] > sort[j - 1]) {
> - tmp = sort[j];
> - sort[j] = sort[j - 1];
> - sort[j - 1] = tmp;
> - }
> - }
> - }
> + s16 sorted_nfval[ATH5K_NF_CAL_HIST_MAX];
> + int i;
> +
> + memcpy(sorted_nfval, ah->ah_nfcal_hist.nfval, sizeof(sorted_nfval));
> + sort(sorted_nfval, ATH5K_NF_CAL_HIST_MAX, sizeof(s16), cmps16, NULL);
> for (i = 0; i < ATH5K_NF_CAL_HIST_MAX; i++) {
> ATH5K_DBG(ah, ATH5K_DEBUG_CALIBRATE,
> - "cal %d:%d\n", i, sort[i]);
> + "cal %d:%d\n", i, sorted_nfval[i]);
> }
> - return sort[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
> + return sorted_nfval[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
> }
>
> /**
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v1] ath5k: fix WARNING opportunity for swap.
2023-08-14 22:28 ` Jeff Johnson
@ 2023-08-14 22:31 ` Jeff Johnson
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Johnson @ 2023-08-14 22:31 UTC (permalink / raw)
To: Mahmoud Maatuq, jirislaby, mickflemm, mcgrof, kvalo,
linux-wireless, linux-kernel
Cc: linux-kernel-mentees
On 8/14/2023 3:28 PM, Jeff Johnson wrote:
> On 8/14/2023 1:02 PM, Mahmoud Maatuq wrote:
>> coccinielle reported the following:
>> ./drivers/net/wireless/ath/ath5k/phy.c:1573:25-26: WARNING opportunity
>> for swap()
>
> Suggest you add something like:
>
> This revealed that ath5k_hw_get_median_noise_floor() had open-coded
> sort() functionality. Since ath5k_hw_get_median_noise_floor() only
> executes once every 10 seconds, any extra overhead due to sort() calling
> it's "compare" and "swap" functions can be ignored, so replace the
<blush> and of course that should be its and not it's </blush>
> existing logic with a call to sort().
>
> and before your SOB add:
> Suggested-by: Jiri Slaby <jirislaby@kernel.org>
>
>> Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
>> ---
>> changes in v1:
>> - replace the entire double loop with sort()
>> as suggested by Jiri Slaby <jirislaby@kernel.org>
>> ---
>> drivers/net/wireless/ath/ath5k/phy.c | 29 +++++++++++++---------------
>> 1 file changed, 13 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath5k/phy.c
>> b/drivers/net/wireless/ath/ath5k/phy.c
>> index 5797ef9c73d7..7ee4e1616f45 100644
>> --- a/drivers/net/wireless/ath/ath5k/phy.c
>> +++ b/drivers/net/wireless/ath/ath5k/phy.c
>> @@ -26,6 +26,7 @@
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> +#include <linux/sort.h>
>> #include <asm/unaligned.h>
>> #include "ath5k.h"
>> @@ -1554,6 +1555,11 @@ static void ath5k_hw_update_nfcal_hist(struct
>> ath5k_hw *ah, s16 noise_floor)
>> hist->nfval[hist->index] = noise_floor;
>> }
>> +static int cmps16(const void *a, const void *b)
>> +{
>> + return *(s16 *)a - *(s16 *)b;
>> +}
>> +
>> /**
>> * ath5k_hw_get_median_noise_floor() - Get median NF from history
>> buffer
>> * @ah: The &struct ath5k_hw
>> @@ -1561,25 +1567,16 @@ static void ath5k_hw_update_nfcal_hist(struct
>> ath5k_hw *ah, s16 noise_floor)
>> static s16
>> ath5k_hw_get_median_noise_floor(struct ath5k_hw *ah)
>> {
>> - s16 sort[ATH5K_NF_CAL_HIST_MAX];
>> - s16 tmp;
>> - int i, j;
>> -
>> - memcpy(sort, ah->ah_nfcal_hist.nfval, sizeof(sort));
>> - for (i = 0; i < ATH5K_NF_CAL_HIST_MAX - 1; i++) {
>> - for (j = 1; j < ATH5K_NF_CAL_HIST_MAX - i; j++) {
>> - if (sort[j] > sort[j - 1]) {
>> - tmp = sort[j];
>> - sort[j] = sort[j - 1];
>> - sort[j - 1] = tmp;
>> - }
>> - }
>> - }
>> + s16 sorted_nfval[ATH5K_NF_CAL_HIST_MAX];
>> + int i;
>> +
>> + memcpy(sorted_nfval, ah->ah_nfcal_hist.nfval, sizeof(sorted_nfval));
>> + sort(sorted_nfval, ATH5K_NF_CAL_HIST_MAX, sizeof(s16), cmps16,
>> NULL);
>> for (i = 0; i < ATH5K_NF_CAL_HIST_MAX; i++) {
>> ATH5K_DBG(ah, ATH5K_DEBUG_CALIBRATE,
>> - "cal %d:%d\n", i, sort[i]);
>> + "cal %d:%d\n", i, sorted_nfval[i]);
>> }
>> - return sort[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
>> + return sorted_nfval[(ATH5K_NF_CAL_HIST_MAX - 1) / 2];
>> }
>> /**
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-14 22:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-14 20:02 [PATCH v1] ath5k: fix WARNING opportunity for swap Mahmoud Maatuq
2023-08-14 22:28 ` Jeff Johnson
2023-08-14 22:31 ` Jeff Johnson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox