All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v1] minmax: fix type warnings
@ 2022-09-13 20:28 Edward Liaw via ltp
  2022-09-14  9:20 ` Petr Vorel
  2022-09-14  9:45 ` Cyril Hrubis
  0 siblings, 2 replies; 10+ messages in thread
From: Edward Liaw via ltp @ 2022-09-13 20:28 UTC (permalink / raw)
  To: ltp; +Cc: kernel-team

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-13 20:28 [LTP] [PATCH v1] minmax: fix type warnings Edward Liaw via ltp
@ 2022-09-14  9:20 ` Petr Vorel
  2022-09-14  9:45 ` Cyril Hrubis
  1 sibling, 0 replies; 10+ messages in thread
From: Petr Vorel @ 2022-09-14  9:20 UTC (permalink / raw)
  To: Edward Liaw; +Cc: kernel-team, ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-13 20:28 [LTP] [PATCH v1] minmax: fix type warnings Edward Liaw via ltp
  2022-09-14  9:20 ` Petr Vorel
@ 2022-09-14  9:45 ` Cyril Hrubis
  2022-09-14 13:35   ` Petr Vorel
  1 sibling, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2022-09-14  9:45 UTC (permalink / raw)
  To: Edward Liaw; +Cc: kernel-team, ltp

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-14  9:45 ` Cyril Hrubis
@ 2022-09-14 13:35   ` Petr Vorel
  2022-09-14 13:50     ` Petr Vorel
  0 siblings, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2022-09-14 13:35 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: kernel-team, ltp

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-14 13:35   ` Petr Vorel
@ 2022-09-14 13:50     ` Petr Vorel
  2022-09-15 20:51       ` Edward Liaw via ltp
  0 siblings, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2022-09-14 13:50 UTC (permalink / raw)
  To: Cyril Hrubis, kernel-team, ltp, Edward Liaw

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-14 13:50     ` Petr Vorel
@ 2022-09-15 20:51       ` Edward Liaw via ltp
  2022-09-16  9:13         ` Cyril Hrubis
  0 siblings, 1 reply; 10+ messages in thread
From: Edward Liaw via ltp @ 2022-09-15 20:51 UTC (permalink / raw)
  To: Petr Vorel; +Cc: kernel-team, 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-15 20:51       ` Edward Liaw via ltp
@ 2022-09-16  9:13         ` Cyril Hrubis
  2022-09-26 18:15           ` Edward Liaw via ltp
  0 siblings, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2022-09-16  9:13 UTC (permalink / raw)
  To: Edward Liaw; +Cc: kernel-team, ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-16  9:13         ` Cyril Hrubis
@ 2022-09-26 18:15           ` Edward Liaw via ltp
  2022-09-27  9:06             ` Cyril Hrubis
  0 siblings, 1 reply; 10+ messages in thread
From: Edward Liaw via ltp @ 2022-09-26 18:15 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: kernel-team, 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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-26 18:15           ` Edward Liaw via ltp
@ 2022-09-27  9:06             ` Cyril Hrubis
  2022-09-27 21:20               ` Edward Liaw via ltp
  0 siblings, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2022-09-27  9:06 UTC (permalink / raw)
  To: Edward Liaw; +Cc: kernel-team, ltp

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v1] minmax: fix type warnings
  2022-09-27  9:06             ` Cyril Hrubis
@ 2022-09-27 21:20               ` Edward Liaw via ltp
  0 siblings, 0 replies; 10+ messages in thread
From: Edward Liaw via ltp @ 2022-09-27 21:20 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: kernel-team, 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

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-27 21:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-13 20:28 [LTP] [PATCH v1] minmax: fix type warnings Edward Liaw via ltp
2022-09-14  9:20 ` Petr Vorel
2022-09-14  9:45 ` Cyril Hrubis
2022-09-14 13:35   ` Petr Vorel
2022-09-14 13:50     ` Petr Vorel
2022-09-15 20:51       ` Edward Liaw via ltp
2022-09-16  9:13         ` Cyril Hrubis
2022-09-26 18:15           ` Edward Liaw via ltp
2022-09-27  9:06             ` Cyril Hrubis
2022-09-27 21:20               ` Edward Liaw via ltp

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.