[PATCH] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[PATCH] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[Dmitry Antipov]

In 'ath9k_hw_get_nf_hist_mid()', prefer 'memcpy()' and 'sort()'
over an ad-hoc things. Briefly tested as a separate module.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/ath/ath9k/calib.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
index 4b331c85509c..26465c1a59e2 100644
--- a/drivers/net/wireless/ath/ath9k/calib.c
+++ b/drivers/net/wireless/ath/ath9k/calib.c
@@ -16,29 +16,25 @@
 
 #include "hw.h"
 #include "hw-ops.h"
+#include <linux/sort.h>
 #include <linux/export.h>
 
 /* Common calibration code */
 
+static inline int rcmp_i16(const void *x, const void *y)
+{
+	/* Sort in reverse order. */
+	return *(int16_t *)y - *(int16_t *)x;
+}
 
 static int16_t ath9k_hw_get_nf_hist_mid(int16_t *nfCalBuffer)
 {
-	int16_t nfval;
-	int16_t sort[ATH9K_NF_CAL_HIST_MAX];
-	int i, j;
-
-	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX; i++)
-		sort[i] = nfCalBuffer[i];
+	int16_t nfcal[ATH9K_NF_CAL_HIST_MAX];
 
-	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
-		for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
-			if (sort[j] > sort[j - 1])
-				swap(sort[j], sort[j - 1]);
-		}
-	}
-	nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
+	memcpy(nfcal, nfCalBuffer, ATH9K_NF_CAL_HIST_MAX * sizeof(int16_t));
+	sort(nfcal, ATH9K_NF_CAL_HIST_MAX, sizeof(int16_t), rcmp_i16, NULL);
 
-	return nfval;
+	return nfcal[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
 }
 
 static struct ath_nf_limits *ath9k_hw_get_nf_limits(struct ath_hw *ah,
-- 
2.47.1

Re: [PATCH] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[Toke Høiland-Jørgensen]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> In 'ath9k_hw_get_nf_hist_mid()', prefer 'memcpy()' and 'sort()'
> over an ad-hoc things. Briefly tested as a separate module.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Thank you for the cleanup; a few nits, see below:

> ---
>  drivers/net/wireless/ath/ath9k/calib.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
> index 4b331c85509c..26465c1a59e2 100644
> --- a/drivers/net/wireless/ath/ath9k/calib.c
> +++ b/drivers/net/wireless/ath/ath9k/calib.c
> @@ -16,29 +16,25 @@
>  
>  #include "hw.h"
>  #include "hw-ops.h"
> +#include <linux/sort.h>
>  #include <linux/export.h>
>  
>  /* Common calibration code */
>  
> +static inline int rcmp_i16(const void *x, const void *y)

We generally don't mark static functions as inline, but instead let the
compiler make the decision (which in this case is to *not* inline, as
the function address needs to be passed to sort()).

> +{
> +	/* Sort in reverse order. */
> +	return *(int16_t *)y - *(int16_t *)x;
> +}
>  
>  static int16_t ath9k_hw_get_nf_hist_mid(int16_t *nfCalBuffer)
>  {
> -	int16_t nfval;
> -	int16_t sort[ATH9K_NF_CAL_HIST_MAX];
> -	int i, j;
> -
> -	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX; i++)
> -		sort[i] = nfCalBuffer[i];
> +	int16_t nfcal[ATH9K_NF_CAL_HIST_MAX];
>  
> -	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
> -		for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
> -			if (sort[j] > sort[j - 1])
> -				swap(sort[j], sort[j - 1]);
> -		}
> -	}
> -	nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
> +	memcpy(nfcal, nfCalBuffer, ATH9K_NF_CAL_HIST_MAX * sizeof(int16_t));

The third argument to memcpy() can just be sizeof(nfcal).


-Toke

[PATCH v2] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[Dmitry Antipov]

In 'ath9k_hw_get_nf_hist_mid()', prefer 'memcpy()' and 'sort()'
over an ad-hoc things. Briefly tested as a separate module.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: as suggested by Toke, do not mark 'rcmp_i16()'
as inline and pass 'sizeof(nfcal)' to 'memcpy()'
---
 drivers/net/wireless/ath/ath9k/calib.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
index 4b331c85509c..b4ab85bd7895 100644
--- a/drivers/net/wireless/ath/ath9k/calib.c
+++ b/drivers/net/wireless/ath/ath9k/calib.c
@@ -16,29 +16,25 @@
 
 #include "hw.h"
 #include "hw-ops.h"
+#include <linux/sort.h>
 #include <linux/export.h>
 
 /* Common calibration code */
 
+static int rcmp_i16(const void *x, const void *y)
+{
+	/* Sort in reverse order. */
+	return *(int16_t *)y - *(int16_t *)x;
+}
 
 static int16_t ath9k_hw_get_nf_hist_mid(int16_t *nfCalBuffer)
 {
-	int16_t nfval;
-	int16_t sort[ATH9K_NF_CAL_HIST_MAX];
-	int i, j;
-
-	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX; i++)
-		sort[i] = nfCalBuffer[i];
+	int16_t nfcal[ATH9K_NF_CAL_HIST_MAX];
 
-	for (i = 0; i < ATH9K_NF_CAL_HIST_MAX - 1; i++) {
-		for (j = 1; j < ATH9K_NF_CAL_HIST_MAX - i; j++) {
-			if (sort[j] > sort[j - 1])
-				swap(sort[j], sort[j - 1]);
-		}
-	}
-	nfval = sort[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
+	memcpy(nfcal, nfCalBuffer, sizeof(nfcal));
+	sort(nfcal, ATH9K_NF_CAL_HIST_MAX, sizeof(int16_t), rcmp_i16, NULL);
 
-	return nfval;
+	return nfcal[(ATH9K_NF_CAL_HIST_MAX - 1) >> 1];
 }
 
 static struct ath_nf_limits *ath9k_hw_get_nf_limits(struct ath_hw *ah,
-- 
2.47.1

Re: [PATCH v2] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[Toke Høiland-Jørgensen]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> In 'ath9k_hw_get_nf_hist_mid()', prefer 'memcpy()' and 'sort()'
> over an ad-hoc things. Briefly tested as a separate module.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>

Re: [PATCH v2] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()

[Jeff Johnson]

On Thu, 09 Jan 2025 11:07:03 +0300, Dmitry Antipov wrote:
> In 'ath9k_hw_get_nf_hist_mid()', prefer 'memcpy()' and 'sort()'
> over an ad-hoc things. Briefly tested as a separate module.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> 

Applied, thanks!

[1/1] wifi: ath9k: cleanup ath9k_hw_get_nf_hist_mid()
      commit: a72eaa175656836a32e94004ea05598409e2f2fd

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>