[v1] minmax: fix type warnings

Several min/max comparisons are missing type conversions.

Signed-off-by: Edward Liaw <edliaw@google.com>
---
 lib/tst_memutils.c                               | 2 +-
 testcases/kernel/mem/mmapstress/mmapstress01.c   | 2 +-
 testcases/kernel/mem/mmapstress/mmapstress10.c   | 2 +-
 testcases/kernel/mem/tunable/overcommit_memory.c | 6 +++---
 testcases/kernel/syscalls/mprotect/mprotect02.c  | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

Hi Edward,

Ah, 32 bit fixes. Thanks!

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

Kind regards,
Petr

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

We can define the min_free to be size_t from the start as:

diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index 0d20bb17c..6fc9f6a93 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -20,11 +20,11 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
 {
        size_t i, map_count = 0, safety = 0, blocksize = BLOCKSIZE;
        unsigned long long freeram;
-       unsigned long min_free;
+       size_t min_free;
        void **map_blocks;
        struct sysinfo info;

-       SAFE_FILE_SCANF("/proc/sys/vm/min_free_kbytes", "%lu", &min_free);
+       SAFE_FILE_SCANF("/proc/sys/vm/min_free_kbytes", "%zu", &min_free);
        min_free *= 1024;
        /* Apply a margin because we cannot get below "min" watermark */
        min_free += min_free / 10;


>  	if (info.freeswap > safety)
> diff --git a/testcases/kernel/mem/mmapstress/mmapstress01.c b/testcases/kernel/mem/mmapstress/mmapstress01.c
> index c16b50a6d..f425c223d 100644
> --- a/testcases/kernel/mem/mmapstress/mmapstress01.c
> +++ b/testcases/kernel/mem/mmapstress/mmapstress01.c
> @@ -310,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, (int)bytes_left);
>  		if ((c = write(fd, buf, write_cnt)) != write_cnt) {
>  			if (c == -1) {
>  				perror("write error");
> diff --git a/testcases/kernel/mem/mmapstress/mmapstress10.c b/testcases/kernel/mem/mmapstress/mmapstress10.c
> index 28b4f1e91..53f02c1f6 100644
> --- a/testcases/kernel/mem/mmapstress/mmapstress10.c
> +++ b/testcases/kernel/mem/mmapstress/mmapstress10.c
> @@ -360,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, (int)bytes_left);
>  		if ((c = write(fd, (char *)buf, write_cnt)) != write_cnt) {
>  			if (c == -1) {
>  				perror("write error");
> diff --git a/testcases/kernel/mem/tunable/overcommit_memory.c b/testcases/kernel/mem/tunable/overcommit_memory.c
> index 20151ebb0..7fe8fe14c 100644
> --- a/testcases/kernel/mem/tunable/overcommit_memory.c
> +++ b/testcases/kernel/mem/tunable/overcommit_memory.c
> @@ -248,9 +248,9 @@ static void calculate_total_batch_size(void)
>  	SAFE_SYSINFO(&info);
>  
>  	/* see linux source mm/mm_init.c mm_compute_batch() (This is in pages) */
> -	long batch_size = MAX(ncpus * 2,
> -	                      MAX(32,
> -	                          MIN(INT32_MAX,
> +	long batch_size = MAX(ncpus * 2L,
> +	                      MAX(32L,
> +	                          MIN((long)INT32_MAX,
>  	                              (long)(info.totalram / pagesize) / ncpus / 256
>  	                          )
>  	                      )
> diff --git a/testcases/kernel/syscalls/mprotect/mprotect02.c b/testcases/kernel/syscalls/mprotect/mprotect02.c
> index de9b4ea00..da445d442 100644
> --- a/testcases/kernel/syscalls/mprotect/mprotect02.c
> +++ b/testcases/kernel/syscalls/mprotect/mprotect02.c
> @@ -77,7 +77,7 @@ int main(int ac, char **av)
>  
>  		do {
>  
> -			bytes_to_write = MIN(strlen(buf), num_bytes);
> +			bytes_to_write = MIN((unsigned int)strlen(buf), num_bytes);

Again we can define the num_bytes to be size_t from the start.


The rest looks good.

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

> We can define the min_free to be size_t from the start as:
Indeed, that's much better. And the second one as well.
Going to merged with these fixes.

Kind regards,
Petr

Hi Edward, Cyril,

Merged with requested fixes + update printf format (SAFE_FILE_SCANF) to %zi.

Kind regards,
Petr

Hey Petr, Cyril,

I'm second-guessing how to handle the off_t and int comparisons in
mem/mmapstress01 and 10.  Would it have been better to do the following?

diff --git a/testcases/kernel/mem/mmapstress/mmapstress01.c
b/testcases/kernel/mem/mmapstress/mmapstress01.c
index f425c223d..934e83006 100644
--- a/testcases/kernel/mem/mmapstress/mmapstress01.c
+++ b/testcases/kernel/mem/mmapstress/mmapstress01.c
@@ -143,7 +143,6 @@ int main(int argc, char *argv[])
        pid_t pid;
        uchar_t *buf = NULL;
        unsigned int seed;
-       int pagesize = sysconf(_SC_PAGE_SIZE);
        float alarmtime = 0;
        struct sigaction sa;
        unsigned i;
@@ -154,8 +153,10 @@ int main(int argc, char *argv[])
        time_t t;
 #ifdef LARGE_FILE
        off64_t bytes_left;
+       off64_t pagesize = sysconf(_SC_PAGE_SIZE);
 #else /* LARGE_FILE */
        off_t bytes_left;
+       off_t pagesize = sysconf(_SC_PAGE_SIZE);
 #endif /* LARGE_FILE */
        const char *filename = "mmapstress01.out";

@@ -310,7 +311,7 @@ int main(int argc, char *argv[])
                anyfail();
        }
        for (bytes_left = filesize; bytes_left; bytes_left -= c) {
-               write_cnt = MIN(pagesize, (int)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/mmapstress10.c
b/testcases/kernel/mem/mmapstress/mmapstress10.c
index 53f02c1f6..1756f7081 100644
--- a/testcases/kernel/mem/mmapstress/mmapstress10.c
+++ b/testcases/kernel/mem/mmapstress/mmapstress10.c
@@ -171,7 +171,6 @@ int main(int argc, char *argv[])
        pid_t wr_pid = 0;
        uchar_t *buf = NULL;
        unsigned int seed;
-       int pagesize = sysconf(_SC_PAGE_SIZE);
        float alarmtime = 0;
        struct sigaction sa;
        unsigned i;
@@ -182,8 +181,10 @@ int main(int argc, char *argv[])
        time_t t;
 #ifdef LARGE_FILE
        off64_t bytes_left;
+       off64_t pagesize = sysconf(_SC_PAGE_SIZE);
 #else /* LARGE_FILE */
        off_t bytes_left;
+       off_t pagesize = sysconf(_SC_PAGE_SIZE);
 #endif /* LARGE_FILE */

        progname = *argv;
@@ -360,7 +361,7 @@ int main(int argc, char *argv[])
        }

        for (bytes_left = filesize; bytes_left; bytes_left -= c) {
-               write_cnt = MIN(pagesize, (int)bytes_left);
+               write_cnt = MIN(pagesize, bytes_left);
                if ((c = write(fd, (char *)buf, write_cnt)) != write_cnt) {
                        if (c == -1) {
                                perror("write error");

Thanks,
Edward

Hi!
Ideally when you are touching that code it should be cleaned up first
and ported to the new test API. The code wasn't properly ported to LTP
to begin with and there are hacks and wrappers around LTP API functions
that should be removed as well.

On Fri, Sep 16, 2022 at 2:11 AM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> Ideally when you are touching that code it should be cleaned up first
> and ported to the new test API. The code wasn't properly ported to LTP
> to begin with and there are hacks and wrappers around LTP API functions
> that should be removed as well.
>
> --
> Cyril Hrubis
> chrubis@suse.cz


Hi Cyril,
Sure thing, I've made an attempt to port mmapstress01, will send that for
feedback before tackling the others.
Thanks,
Edward

Hi!
> Sure thing, I've made an attempt to port mmapstress01, will send that for
> feedback before tackling the others.

We are finalizing a LTP release right now, I will have a look once I
have a bit of time, however it probably wouldn't be sooner than next
week.

Hi Cyril,
On Tue, Sep 27, 2022 at 2:05 AM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > Sure thing, I've made an attempt to port mmapstress01, will send that for
> > feedback before tackling the others.
>
> We are finalizing a LTP release right now, I will have a look once I
> have a bit of time, however it probably wouldn't be sooner than next
> week.
>

Sounds good, thanks!