[LTP] [PATCH 0/3] refine the MIN/MAX macros

[LTP] [PATCH 0/3] refine the MIN/MAX macros

[Li Wang]

Add type checking to MIN/MAX and make use of them for both
new and old testcases.

Li Wang (3):
  include: replace min/max macro by the new definition
  lib: adding unnecessary pointer comparison
  minmax: ensure the comparison numbers have compatible type

 include/ipcmsg.h                                      | 2 --
 include/libnewipc.h                                   | 1 -
 include/tst_minmax.h                                  | 2 ++
 lib/tst_cgroup.c                                      | 2 +-
 lib/tst_memutils.c                                    | 4 ++--
 lib/tst_timer_test.c                                  | 4 ++--
 testcases/kernel/mem/mmapstress/mmapstress01.c        | 3 +--
 testcases/kernel/mem/mmapstress/mmapstress09.c        | 1 -
 testcases/kernel/mem/mmapstress/mmapstress10.c        | 3 +--
 testcases/kernel/security/cap_bound/cap_bounds_r.c    | 3 +--
 testcases/kernel/security/cap_bound/cap_bounds_rw.c   | 7 +++----
 testcases/kernel/syscalls/fallocate/fallocate06.c     | 2 +-
 testcases/kernel/syscalls/ipc/msgstress/msgstress01.c | 2 +-
 testcases/kernel/syscalls/ipc/msgstress/msgstress02.c | 2 +-
 14 files changed, 16 insertions(+), 22 deletions(-)

-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] include: replace min/max macro by the new definition

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/ipcmsg.h                                      | 2 --
 include/libnewipc.h                                   | 1 -
 testcases/kernel/mem/mmapstress/mmapstress01.c        | 3 +--
 testcases/kernel/mem/mmapstress/mmapstress09.c        | 1 -
 testcases/kernel/mem/mmapstress/mmapstress10.c        | 3 +--
 testcases/kernel/security/cap_bound/cap_bounds_r.c    | 3 +--
 testcases/kernel/security/cap_bound/cap_bounds_rw.c   | 7 +++----
 testcases/kernel/syscalls/ipc/msgstress/msgstress01.c | 2 +-
 testcases/kernel/syscalls/ipc/msgstress/msgstress02.c | 2 +-
 9 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/ipcmsg.h b/include/ipcmsg.h
index d89894b72..3b3fa32c0 100644
--- a/include/ipcmsg.h
+++ b/include/ipcmsg.h
@@ -43,8 +43,6 @@ void setup(void);
 
 #define NR_MSGQUEUES	16	/* MSGMNI as defined in linux/msg.h */
 
-#define min(a, b)	(((a) < (b)) ? (a) : (b))
-
 typedef struct mbuf {		/* a generic message structure */
 	long mtype;
 	char mtext[MSGSIZE + 1];  /* add 1 here so the message can be 1024   */
diff --git a/include/libnewipc.h b/include/libnewipc.h
index 9eec31763..1e126ca1c 100644
--- a/include/libnewipc.h
+++ b/include/libnewipc.h
@@ -31,7 +31,6 @@
 #define MSGSIZE	1024
 #define MSGTYPE	1
 #define NR_MSGQUEUES	16
-#define min(a, b)	(((a) < (b)) ? (a) : (b))
 
 #define SEM_RD	0400
 #define SEM_ALT	0200
diff --git a/testcases/kernel/mem/mmapstress/mmapstress01.c b/testcases/kernel/mem/mmapstress/mmapstress01.c
index 3b4b1ac24..c16b50a6d 100644
--- a/testcases/kernel/mem/mmapstress/mmapstress01.c
+++ b/testcases/kernel/mem/mmapstress/mmapstress01.c
@@ -95,7 +95,6 @@ void ok_exit();
 #undef roundup
 #endif
 #define roundup(x, y)	((((x)+((y)-1))/(y))*(y))
-#define min(x, y)	(((x) < (y)) ? (x) : (y))
 
 extern time_t time(time_t *);
 extern char *ctime(const time_t *);
@@ -311,7 +310,7 @@ int main(int argc, char *argv[])
 		anyfail();
 	}
 	for (bytes_left = filesize; bytes_left; bytes_left -= c) {
-		write_cnt = min(pagesize, bytes_left);
+		write_cnt = MIN(pagesize, bytes_left);
 		if ((c = write(fd, buf, write_cnt)) != write_cnt) {
 			if (c == -1) {
 				perror("write error");
diff --git a/testcases/kernel/mem/mmapstress/mmapstress09.c b/testcases/kernel/mem/mmapstress/mmapstress09.c
index 2c710df1e..0a8da0006 100644
--- a/testcases/kernel/mem/mmapstress/mmapstress09.c
+++ b/testcases/kernel/mem/mmapstress/mmapstress09.c
@@ -78,7 +78,6 @@ void ok_exit();
 #undef roundup
 #endif
 #define roundup(x, y)	((((x)+((y)-1))/(y))*(y))
-#define min(x, y)	(((x) < (y)) ? (x) : (y))
 
 extern time_t time(time_t *);
 extern char *ctime(const time_t *);
diff --git a/testcases/kernel/mem/mmapstress/mmapstress10.c b/testcases/kernel/mem/mmapstress/mmapstress10.c
index 26ea98bc1..28b4f1e91 100644
--- a/testcases/kernel/mem/mmapstress/mmapstress10.c
+++ b/testcases/kernel/mem/mmapstress/mmapstress10.c
@@ -106,7 +106,6 @@ void ok_exit();
 #undef roundup
 #endif
 #define roundup(x, y)	((((x)+((y)-1))/(y))*(y))
-#define min(x, y)	(((x) < (y)) ? (x) : (y))
 
 #define SIZE_MAX UINT_MAX
 
@@ -361,7 +360,7 @@ int main(int argc, char *argv[])
 	}
 
 	for (bytes_left = filesize; bytes_left; bytes_left -= c) {
-		write_cnt = min(pagesize, bytes_left);
+		write_cnt = MIN(pagesize, bytes_left);
 		if ((c = write(fd, (char *)buf, write_cnt)) != write_cnt) {
 			if (c == -1) {
 				perror("write error");
diff --git a/testcases/kernel/security/cap_bound/cap_bounds_r.c b/testcases/kernel/security/cap_bound/cap_bounds_r.c
index d7c2bf0ae..28f320fd9 100644
--- a/testcases/kernel/security/cap_bound/cap_bounds_r.c
+++ b/testcases/kernel/security/cap_bound/cap_bounds_r.c
@@ -85,9 +85,8 @@ int main(void)
 	 * We could test using kernel API, but that's what we're
 	 * testing...  So let's take an insanely high value */
 #define INSANE 63
-#define max(x,y) (x > y ? x : y)
 #if HAVE_DECL_PR_CAPBSET_READ
-	ret = prctl(PR_CAPBSET_READ, max(INSANE, CAP_LAST_CAP + 1));
+	ret = prctl(PR_CAPBSET_READ, MAX(INSANE, CAP_LAST_CAP + 1));
 #else
 	errno = ENOSYS;
 	ret = -1;
diff --git a/testcases/kernel/security/cap_bound/cap_bounds_rw.c b/testcases/kernel/security/cap_bound/cap_bounds_rw.c
index 503853c5b..a0d2111d6 100644
--- a/testcases/kernel/security/cap_bound/cap_bounds_rw.c
+++ b/testcases/kernel/security/cap_bound/cap_bounds_rw.c
@@ -115,18 +115,17 @@ int main(void)
 	 * We could test using kernel API, but that's what we're
 	 * testing...  So let's take an insanely high value */
 #define INSANE 63
-#define max(x,y) (x > y ? x : y)
 #if HAVE_DECL_PR_CAPBSET_DROP
-	ret = prctl(PR_CAPBSET_DROP, max(INSANE, CAP_LAST_CAP + 1));
+	ret = prctl(PR_CAPBSET_DROP, MAX(INSANE, CAP_LAST_CAP + 1));
 #else
 	errno = ENOSYS;
 	ret = -1;
 #endif
 	if (ret != -1) {
 		tst_resm(TFAIL, "prctl(PR_CAPBSET_DROP, %d) returned %d",
-			 max(INSANE, CAP_LAST_CAP + 1), ret);
+			 MAX(INSANE, CAP_LAST_CAP + 1), ret);
 		tst_resm(TINFO, " %d is should not exist",
-			 max(INSANE, CAP_LAST_CAP + 1));
+			 MAX(INSANE, CAP_LAST_CAP + 1));
 		tst_exit();
 	}
 	for (i = 0; i <= cap_last_cap; i++) {
diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
index 0a660c042..84e338437 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
@@ -280,7 +280,7 @@ void setup(void)
 	 * that are not necessary for this test.
 	 * That's why we define NR_MSGQUEUES as a high boundary for it.
 	 */
-	MSGMNI = min(nr_msgqs, NR_MSGQUEUES);
+	MSGMNI = MIN(nr_msgqs, NR_MSGQUEUES);
 }
 
 void cleanup(void)
diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress02.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress02.c
index e15131043..a0f894b05 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress02.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress02.c
@@ -385,7 +385,7 @@ void setup(void)
 	 * that are not necessary for this test.
 	 * That's why we define NR_MSGQUEUES as a high boundary for it.
 	 */
-	MSGMNI = min(nr_msgqs, NR_MSGQUEUES);
+	MSGMNI = MIN(nr_msgqs, NR_MSGQUEUES);
 }
 
 void cleanup(void)
-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] lib: adding unnecessary pointer comparison

[Li Wang]

The intention is to ensure that _a and _b have compatible type by
comparing their addresses; only if _a and _b have compatible types
will pointers to them have compatible type. The result of the
comparison is ignored, so the only effect is to provoke a diagnostic
from the compiler if _a and _b have incompatible types.

The goal is to avoid taking the minimum/maximum of values of different
types, because the result can be surprising.

Btw, I don't think we need to port the latest version of min/max from
linux/include/linux/minmax.h because it has an additional constant
expression checker. Actually, we (LTP) do have some tests pass variable
to MIN/MAX and it should _not_ be working at that senario.

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Chunyu Hu <chuhu@redhat.com>
---
 include/tst_minmax.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/tst_minmax.h b/include/tst_minmax.h
index 6417dd703..9d7d596fc 100644
--- a/include/tst_minmax.h
+++ b/include/tst_minmax.h
@@ -9,6 +9,7 @@
 # define MIN(a, b) ({ \
 	typeof(a) _a = (a); \
 	typeof(b) _b = (b); \
+	(void) (&_a == &_b); \
 	_a < _b ? _a : _b; \
 })
 #endif /* MIN */
@@ -17,6 +18,7 @@
 # define MAX(a, b) ({ \
 	typeof(a) _a = (a); \
 	typeof(b) _b = (b); \
+	(void) (&_a == &_b); \
 	_a > _b ? _a : _b; \
 })
 #endif /* MAX */
-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] minmax: ensure the comparison numbers have compatible type

[Li Wang]

To get rid of many compiler warnings.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 lib/tst_cgroup.c                                  | 2 +-
 lib/tst_memutils.c                                | 4 ++--
 lib/tst_timer_test.c                              | 4 ++--
 testcases/kernel/syscalls/fallocate/fallocate06.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 8cca0654d..57940ba09 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1116,7 +1116,7 @@ ssize_t safe_cg_read(const char *const file, const int lineno,
 		prev_len = MIN(sizeof(prev_buf), (size_t)read_ret);
 	}
 
-	out[MAX(read_ret, 0)] = '\0';
+	out[MAX(read_ret, (ssize_t)0)] = '\0';
 
 	return read_ret;
 }
diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index 3741d6e6f..d95d75e5d 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -30,7 +30,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
 	min_free += min_free / 10;
 
 	SAFE_SYSINFO(&info);
-	safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
+	safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), (long)128 * 1024 * 1024);
 	safety = MAX(safety, min_free);
 	safety /= info.mem_unit;
 
@@ -44,7 +44,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
 	 * Use the lower value of both for pollutable memory. Usually this
 	 * means we will not evict any caches.
 	 */
-	freeram = MIN(info.freeram, (tst_available_mem() * 1024));
+	freeram = MIN((long long)info.freeram, (tst_available_mem() * 1024));
 
 	/* Not enough free memory to avoid invoking OOM killer */
 	if (freeram <= safety)
diff --git a/lib/tst_timer_test.c b/lib/tst_timer_test.c
index 3cd52fc9d..c0e3de3e7 100644
--- a/lib/tst_timer_test.c
+++ b/lib/tst_timer_test.c
@@ -67,7 +67,7 @@ static const char *table_heading = " Time: us ";
  */
 static unsigned int header_len(long long max_sample)
 {
-	unsigned int l = 1;
+	size_t l = 1;
 
 	while (max_sample/=10)
 		l++;
@@ -184,7 +184,7 @@ static int cmp(const void *a, const void *b)
 static long long compute_threshold(long long requested_us,
 				   unsigned int nsamples)
 {
-	unsigned int slack_per_scall = MIN(100000, requested_us / 1000);
+	unsigned int slack_per_scall = MIN((long long)100000, requested_us / 1000);
 
 	slack_per_scall = MAX(slack_per_scall, timerslack);
 
diff --git a/testcases/kernel/syscalls/fallocate/fallocate06.c b/testcases/kernel/syscalls/fallocate/fallocate06.c
index bf0d24328..25b27e41d 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate06.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate06.c
@@ -95,7 +95,7 @@ static void setup(void)
 	TEST(toggle_cow(fd, 0));
 	SAFE_FSTAT(fd, &statbuf);
 	blocksize = statbuf.st_blksize;
-	block_offset = MIN(blocksize / 2, 512);
+	block_offset = MIN(blocksize / 2, (blksize_t)512);
 	wbuf_size = MAX(WRITE_BLOCKS, FALLOCATE_BLOCKS) * blocksize;
 	rbuf_size = (DEALLOCATE_BLOCKS + 1) * blocksize;
 	SAFE_CLOSE(fd);
-- 
2.31.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] minmax: ensure the comparison numbers have compatible type

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 2403 bytes --]

On Fri, Mar 11, 2022 at 1:46 PM Li Wang <liwang@redhat.com> wrote:

> To get rid of many compiler warnings.
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  lib/tst_cgroup.c                                  | 2 +-
>  lib/tst_memutils.c                                | 4 ++--
>  lib/tst_timer_test.c                              | 4 ++--
>  testcases/kernel/syscalls/fallocate/fallocate06.c | 2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 8cca0654d..57940ba09 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -1116,7 +1116,7 @@ ssize_t safe_cg_read(const char *const file, const
> int lineno,
>                 prev_len = MIN(sizeof(prev_buf), (size_t)read_ret);
>         }
>
> -       out[MAX(read_ret, 0)] = '\0';
> +       out[MAX(read_ret, (ssize_t)0)] = '\0';
>
>         return read_ret;
>  }
> diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
> index 3741d6e6f..d95d75e5d 100644
> --- a/lib/tst_memutils.c
> +++ b/lib/tst_memutils.c
> @@ -30,7 +30,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
>         min_free += min_free / 10;
>
>         SAFE_SYSINFO(&info);
> -       safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128 * 1024 * 1024);
> +       safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), (long)128 * 1024 *
> 1024);
>

This should be 128L. and tst_timer_test has the same issue.

--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -30,7 +30,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
        min_free += min_free / 10;

        SAFE_SYSINFO(&info);
-       safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), (long)128 * 1024 *
1024);
+       safety = MAX(4096 * SAFE_SYSCONF(_SC_PAGESIZE), 128L * 1024 * 1024);
        safety = MAX(safety, min_free);
        safety /= info.mem_unit;

diff --git a/lib/tst_timer_test.c b/lib/tst_timer_test.c
index c0e3de3e7..32fa55c7c 100644
--- a/lib/tst_timer_test.c
+++ b/lib/tst_timer_test.c
@@ -184,7 +184,7 @@ static int cmp(const void *a, const void *b)
 static long long compute_threshold(long long requested_us,
                                   unsigned int nsamples)
 {
-       unsigned int slack_per_scall = MIN((long long)100000, requested_us
/ 1000);
+       unsigned int slack_per_scall = MIN(100000LL, requested_us / 1000);

        slack_per_scall = MAX(slack_per_scall, timerslack);


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3467 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] include: replace min/max macro by the new definition

[Petr Vorel]

Hi Li,

obviously correct, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib: adding unnecessary pointer comparison

[Petr Vorel]

Hi Li,

looks reasonable.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] minmax: ensure the comparison numbers have compatible type

[Petr Vorel]

Hi Li,

> > To get rid of many compiler warnings.
A bit uncomfortable, but nothing problematic.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] include: replace min/max macro by the new definition

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 203 bytes --]

On Wed, Mar 23, 2022 at 6:19 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> obviously correct, thanks!
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>

Patchset applied, thanks!


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 826 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp