diff mbox series

[v2,1/2] selftests: Provide local define of min() and max()

Message ID 20230824202415.131824-1-mahmoudmatook.mm@gmail.com
State New
Headers show
Series [v2,1/2] selftests: Provide local define of min() and max() | expand

Commit Message

Mahmoud Maatuq Aug. 24, 2023, 8:24 p.m. UTC
to avoid manual calculation of min and max values
and fix coccinelle warnings such WARNING opportunity for min()/max()
adding one common definition that could be used in multiple files
under selftests.
there are also some defines for min/max scattered locally inside sources
under selftests.
this also prepares for cleaning up those redundant defines and include
kselftest.h instead.

Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
Suggested-by: David Laight <David.Laight@aculab.com>
---
changes in v2:
redefine min/max in a more strict way to avoid 
signedness mismatch and multiple evaluation.
is_signed_type() moved from selftests/kselftest_harness.h 
to selftests/kselftest.h.
---
 tools/testing/selftests/kselftest.h         | 24 +++++++++++++++++++++
 tools/testing/selftests/kselftest_harness.h |  2 --
 2 files changed, 24 insertions(+), 2 deletions(-)

Comments

Sean Christopherson Aug. 25, 2023, 3:26 p.m. UTC | #1
On Fri, Aug 25, 2023, Mahmoud Maatuq wrote:
> to avoid manual calculation of min and max values
> and fix coccinelle warnings such WARNING opportunity for min()/max()
> adding one common definition that could be used in multiple files
> under selftests.
> there are also some defines for min/max scattered locally inside sources
> under selftests.
> this also prepares for cleaning up those redundant defines and include
> kselftest.h instead.
> 
> Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
> Suggested-by: David Laight <David.Laight@aculab.com>
> ---
> changes in v2:
> redefine min/max in a more strict way to avoid 
> signedness mismatch and multiple evaluation.
> is_signed_type() moved from selftests/kselftest_harness.h 
> to selftests/kselftest.h.
> ---
>  tools/testing/selftests/kselftest.h         | 24 +++++++++++++++++++++

Heh, reminds me of https://xkcd.com/927.

All of these #defines are available in tools/include/linux/kernel.h, and it's
trivially easy for selftests to add all of tools/include to their include path.
I don't see any reason for the selftests framework to define yet another version,
just fix the individual tests.
Mahmoud Maatuq Aug. 26, 2023, 9:45 a.m. UTC | #2
On 08/25, Sean Christopherson wrote:

> On Fri, Aug 25, 2023, Mahmoud Maatuq wrote:
> > to avoid manual calculation of min and max values
> > and fix coccinelle warnings such WARNING opportunity for min()/max()
> > adding one common definition that could be used in multiple files
> > under selftests.
> > there are also some defines for min/max scattered locally inside sources
> > under selftests.
> > this also prepares for cleaning up those redundant defines and include
> > kselftest.h instead.
> > 
> > Signed-off-by: Mahmoud Maatuq <mahmoudmatook.mm@gmail.com>
> > Suggested-by: David Laight <David.Laight@aculab.com>
> > ---
> > changes in v2:
> > redefine min/max in a more strict way to avoid 
> > signedness mismatch and multiple evaluation.
> > is_signed_type() moved from selftests/kselftest_harness.h 
> > to selftests/kselftest.h.
> > ---
> >  tools/testing/selftests/kselftest.h         | 24 +++++++++++++++++++++
> 
> Heh, reminds me of https://xkcd.com/927.
> 
> All of these #defines are available in tools/include/linux/kernel.h, and it's
> trivially easy for selftests to add all of tools/include to their include path.
> I don't see any reason for the selftests framework to define yet another version,
> just fix the individual tests.

giving the reviews seems that patchset is useless.
still a confusing point for me; after adding tools/include to the
include path of selftes how we can differentaite between  #include
<linux/kernel.h> that under tools/include and one under usr/include.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index 829be379545a..93d029471cc9 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -55,6 +55,30 @@ 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 #endif
 
+#ifndef is_signed_type
+#define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
+#endif
+
+#ifndef min
+#define min(x, y) ({ \
+	_Static_assert(is_signed_type(typeof(x)) == is_signed_type(typeof(y)), \
+	       "min: signedness mismatch"); \
+	typeof(x) _x = (x); \
+	typeof(y) _y = (y); \
+	_x < _y ? _x : _y; \
+})
+#endif
+
+#ifndef max
+#define max(x, y) ({ \
+	_Static_assert(is_signed_type(typeof(x)) == is_signed_type(typeof(y)), \
+	       "max: signedness mismatch"); \
+	typeof(x) _x = (x); \
+	typeof(y) _y = (y); \
+	_x > _y ? _x : _y; \
+})
+#endif
+
 /*
  * gcc cpuid.h provides __cpuid_count() since v4.4.
  * Clang/LLVM cpuid.h provides  __cpuid_count() since v3.4.0.
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 5fd49ad0c696..e4e310815226 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -699,8 +699,6 @@ 
 	if (_metadata->passed && _metadata->step < 253) \
 		_metadata->step++;
 
-#define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
-
 #define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
 	/* Avoid multiple evaluation of the cases */ \
 	__typeof__(_expected) __exp = (_expected); \