[LTP] [PATCH v1] minmax: fix type warnings

[LTP] [PATCH v1] minmax: fix type warnings

[Edward Liaw via ltp]

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(-)

diff --git a/lib/tst_memutils.c b/lib/tst_memutils.c
index 0d20bb17c..c6696437d 100644
--- a/lib/tst_memutils.c
+++ b/lib/tst_memutils.c
@@ -31,7 +31,7 @@ void tst_pollute_memory(size_t maxsize, int fillchar)
 
 	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;
 
 	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);
 
 			num_bytes -=
 			    SAFE_WRITE(cleanup, 1, fd, buf, bytes_to_write);
-- 
2.37.2.789.g6183377224-goog


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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Petr Vorel]

Hi Edward,

Ah, 32 bit fixes. Thanks!

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

Kind regards,
Petr

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Petr Vorel]

> 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

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Petr Vorel]

Hi Edward, Cyril,

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

Kind regards,
Petr

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Edward Liaw via ltp]

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

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

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

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


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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Cyril Hrubis]

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

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Edward Liaw via ltp]

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

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

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

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


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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] minmax: fix type warnings

[Edward Liaw via ltp]

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

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!

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

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


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