| Message ID | 20241108134920.1233992-3-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kselftest/arm64: Fix compilation warnings/errors in the arm64 tests | expand |
On Fri, Nov 08, 2024 at 01:49:18PM +0000, Catalin Marinas wrote: > While prctl() returns an 'int', the PR_MTE_TCF_MASK is defined as > unsigned long which results in the larger type following a bitwise 'and' > operation. Cast the printf() argument to 'int'. > } else { > ksft_print_msg("Got %x, expected %x\n", > - (ret & PR_MTE_TCF_MASK), mask); > + (int)(ret & PR_MTE_TCF_MASK), mask); Shouldn't we just use a %lx here? Casts tend to be suspicious...
On Fri, Nov 08, 2024 at 03:10:59PM +0000, Mark Brown wrote: > On Fri, Nov 08, 2024 at 01:49:18PM +0000, Catalin Marinas wrote: > > While prctl() returns an 'int', the PR_MTE_TCF_MASK is defined as > > unsigned long which results in the larger type following a bitwise 'and' > > operation. Cast the printf() argument to 'int'. > > > } else { > > ksft_print_msg("Got %x, expected %x\n", > > - (ret & PR_MTE_TCF_MASK), mask); > > + (int)(ret & PR_MTE_TCF_MASK), mask); > > Shouldn't we just use a %lx here? Casts tend to be suspicious... It's more like the ret is actually 32-bit and should stay like that when bits are masked out. But the bitwise op 'upgrades' it to a long (in hindsight, we should not have used UL for the TCF bits and mask).
On Fri, Nov 08, 2024 at 03:25:46PM +0000, Catalin Marinas wrote: > On Fri, Nov 08, 2024 at 03:10:59PM +0000, Mark Brown wrote: > > > - (ret & PR_MTE_TCF_MASK), mask); > > > + (int)(ret & PR_MTE_TCF_MASK), mask); > > Shouldn't we just use a %lx here? Casts tend to be suspicious... > It's more like the ret is actually 32-bit and should stay like that when > bits are masked out. But the bitwise op 'upgrades' it to a long (in > hindsight, we should not have used UL for the TCF bits and mask). Hrm, right. Possibly put the cast on PR_MTE_TCF_MASK rather than on the expression as a whole then?
On Fri, Nov 08, 2024 at 03:30:11PM +0000, Mark Brown wrote: > On Fri, Nov 08, 2024 at 03:25:46PM +0000, Catalin Marinas wrote: > > On Fri, Nov 08, 2024 at 03:10:59PM +0000, Mark Brown wrote: > > > > > - (ret & PR_MTE_TCF_MASK), mask); > > > > + (int)(ret & PR_MTE_TCF_MASK), mask); > > > > Shouldn't we just use a %lx here? Casts tend to be suspicious... > > > It's more like the ret is actually 32-bit and should stay like that when > > bits are masked out. But the bitwise op 'upgrades' it to a long (in > > hindsight, we should not have used UL for the TCF bits and mask). > > Hrm, right. Possibly put the cast on PR_MTE_TCF_MASK rather than on the > expression as a whole then? Can do.
diff --git a/tools/testing/selftests/arm64/mte/check_prctl.c b/tools/testing/selftests/arm64/mte/check_prctl.c index f139a33a43ef..51e3f41a54d1 100644 --- a/tools/testing/selftests/arm64/mte/check_prctl.c +++ b/tools/testing/selftests/arm64/mte/check_prctl.c @@ -85,7 +85,7 @@ void set_mode_test(const char *name, int hwcap2, int mask) ksft_test_result_pass("%s\n", name); } else { ksft_print_msg("Got %x, expected %x\n", - (ret & PR_MTE_TCF_MASK), mask); + (int)(ret & PR_MTE_TCF_MASK), mask); ksft_test_result_fail("%s\n", name); } }
While prctl() returns an 'int', the PR_MTE_TCF_MASK is defined as unsigned long which results in the larger type following a bitwise 'and' operation. Cast the printf() argument to 'int'. Cc: Shuah Khan <skhan@linuxfoundation.org> Cc: Mark Brown <broonie@kernel.org> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> --- tools/testing/selftests/arm64/mte/check_prctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)