| Message ID | 20240505221359.65258-1-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 727759d748ed34cc8d3e1d215fbc1766010dee3d |
| Headers | show |
| Series | selftests/mqueue: fix 5 warnings about signed/unsigned mismatches | expand |
On 05/05/2024 23:13, John Hubbard wrote: > When building with clang, via: > > make LLVM=1 -C tools/testing/selftest > > ...clang warns about several cases of using a signed integer for the > priority argument to mq_receive(3), which expects an unsigned int. > > Fix this by declaring the type as unsigned int in all cases. > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > tools/testing/selftests/mqueue/mq_perf_tests.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c > index 5c16159d0bcd..fb898850867c 100644 > --- a/tools/testing/selftests/mqueue/mq_perf_tests.c > +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c > @@ -323,7 +323,8 @@ void *fake_cont_thread(void *arg) > void *cont_thread(void *arg) > { > char buff[MSG_SIZE]; > - int i, priority; > + int i; > + unsigned int priority; > > for (i = 0; i < num_cpus_to_pin; i++) > if (cpu_threads[i] == pthread_self()) > @@ -425,7 +426,8 @@ struct test test2[] = { > void *perf_test_thread(void *arg) > { > char buff[MSG_SIZE]; > - int prio_out, prio_in; > + int prio_out; It feels a bit odd for prio_out and prio_in to have different types. I don't have any prior familiararity with these tests but looks like they are ultimately the parameters of mq_send() and mq_receive() which both define them as unsigned ints. Perhaps both should be converted? > + unsigned int prio_in; > int i; > clockid_t clock; > pthread_t *t; > > base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b > prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
On 5/7/24 12:54 AM, Ryan Roberts wrote: > On 05/05/2024 23:13, John Hubbard wrote: ... >> diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c >> index 5c16159d0bcd..fb898850867c 100644 >> --- a/tools/testing/selftests/mqueue/mq_perf_tests.c >> +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c >> @@ -323,7 +323,8 @@ void *fake_cont_thread(void *arg) >> void *cont_thread(void *arg) >> { >> char buff[MSG_SIZE]; >> - int i, priority; >> + int i; >> + unsigned int priority; >> >> for (i = 0; i < num_cpus_to_pin; i++) >> if (cpu_threads[i] == pthread_self()) >> @@ -425,7 +426,8 @@ struct test test2[] = { >> void *perf_test_thread(void *arg) >> { >> char buff[MSG_SIZE]; >> - int prio_out, prio_in; >> + int prio_out; > > It feels a bit odd for prio_out and prio_in to have different types. I don't > have any prior familiararity with these tests but looks like they are ultimately > the parameters of mq_send() and mq_receive() which both define them as unsigned > ints. Perhaps both should be converted? This makes sense, and I recall wondering about it. Looking at it again, I see why didn't go that far: there is a mini-unit test manager inside, passing around priorities that are signed, throughout: struct test { char *desc; void (*func)(int *); }; ... void inc_prio(int *prio) { if (++*prio == mq_prio_max) *prio = 0; } However, I can probably fix up everything to match up. Given that you've called it out, I'll go ahead with that approach. Iit will be quite a few changes but they will all be trivial too. thanks,
On 07/05/2024 18:04, John Hubbard wrote: > On 5/7/24 12:54 AM, Ryan Roberts wrote: >> On 05/05/2024 23:13, John Hubbard wrote: > ... >>> diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c >>> b/tools/testing/selftests/mqueue/mq_perf_tests.c >>> index 5c16159d0bcd..fb898850867c 100644 >>> --- a/tools/testing/selftests/mqueue/mq_perf_tests.c >>> +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c >>> @@ -323,7 +323,8 @@ void *fake_cont_thread(void *arg) >>> void *cont_thread(void *arg) >>> { >>> char buff[MSG_SIZE]; >>> - int i, priority; >>> + int i; >>> + unsigned int priority; >>> for (i = 0; i < num_cpus_to_pin; i++) >>> if (cpu_threads[i] == pthread_self()) >>> @@ -425,7 +426,8 @@ struct test test2[] = { >>> void *perf_test_thread(void *arg) >>> { >>> char buff[MSG_SIZE]; >>> - int prio_out, prio_in; >>> + int prio_out; >> >> It feels a bit odd for prio_out and prio_in to have different types. I don't >> have any prior familiararity with these tests but looks like they are ultimately >> the parameters of mq_send() and mq_receive() which both define them as unsigned >> ints. Perhaps both should be converted? > > > This makes sense, and I recall wondering about it. Looking at it again, > I see why didn't go that far: there is a mini-unit test manager inside, > passing around priorities that are signed, throughout: > > struct test { > char *desc; void (*func)(int *); > }; > > ... > > void inc_prio(int *prio) { > if (++*prio == mq_prio_max) > *prio = 0; > } > > However, I can probably fix up everything to match up. Given that you've > called it out, I'll go ahead with that approach. Iit will be quite a few > changes but they will all be trivial too. Ahh I see. It would certainly be an improvement, but if you don't think it's worth the effort, then don't feel you need to do it on my account. > > > thanks,
diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c index 5c16159d0bcd..fb898850867c 100644 --- a/tools/testing/selftests/mqueue/mq_perf_tests.c +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c @@ -323,7 +323,8 @@ void *fake_cont_thread(void *arg) void *cont_thread(void *arg) { char buff[MSG_SIZE]; - int i, priority; + int i; + unsigned int priority; for (i = 0; i < num_cpus_to_pin; i++) if (cpu_threads[i] == pthread_self()) @@ -425,7 +426,8 @@ struct test test2[] = { void *perf_test_thread(void *arg) { char buff[MSG_SIZE]; - int prio_out, prio_in; + int prio_out; + unsigned int prio_in; int i; clockid_t clock; pthread_t *t;
When building with clang, via: make LLVM=1 -C tools/testing/selftest ...clang warns about several cases of using a signed integer for the priority argument to mq_receive(3), which expects an unsigned int. Fix this by declaring the type as unsigned int in all cases. Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- tools/testing/selftests/mqueue/mq_perf_tests.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) base-commit: f462ae0edd3703edd6f22fe41d336369c38b884b prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27