[PATCH v2] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix()
A recent change in clang allows it to consider more expressions as compile time constants, which causes it to point out an implicit conversion in the scanf tests:
lib/test_scanf.c:661:2: warning: implicit conversion from 'int' to 'unsigned char' changes value from -168 to 88 [-Wconstant-conversion] 661 | test_number_prefix(unsigned char, "0xA7", "%2hhx%hhx", 0, 0xa7, 2, check_uchar); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/test_scanf.c:609:29: note: expanded from macro 'test_number_prefix' 609 | T result[2] = {~expect[0], ~expect[1]}; \ | ~ ^~~~~~~~~~ 1 warning generated.
The result of the bitwise negation is the type of the operand after going through the integer promotion rules, so this truncation is expected but harmless, as the initial values in the result array get overwritten by _test() anyways. Add an explicit cast to the expected type in test_number_prefix() to silence the warning. There is no functional change, as all the tests still pass with GCC 13.1.0 and clang 18.0.0.
Cc: stable@vger.kernel.org Closes: https://github.com/ClangBuiltLinux/linux/issues/1899 Link: https://github.com/llvm/llvm-project/commit/610ec954e1f81c0e8fcadedcd25afe64... Suggested-by: Nick Desaulniers ndesaulniers@google.com Signed-off-by: Nathan Chancellor nathan@kernel.org --- Changes in v2: - Add spaces in initializer to match result (Andy) - Add 'Cc: stable', as builds with CONFIG_WERROR will be broken, such as allmodconfig - Link to v1: https://lore.kernel.org/r/20230803-test_scanf-wconstant-conversion-v1-1-74da... --- lib/test_scanf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_scanf.c b/lib/test_scanf.c index b620cf7de503..a2707af2951a 100644 --- a/lib/test_scanf.c +++ b/lib/test_scanf.c @@ -606,7 +606,7 @@ static void __init numbers_slice(void) #define test_number_prefix(T, str, scan_fmt, expect0, expect1, n_args, fn) \ do { \ const T expect[2] = { expect0, expect1 }; \ - T result[2] = {~expect[0], ~expect[1]}; \ + T result[2] = { (T)~expect[0], (T)~expect[1] }; \ \ _test(fn, &expect, str, scan_fmt, n_args, &result[0], &result[1]); \ } while (0)
--- base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 change-id: 20230803-test_scanf-wconstant-conversion-d97efbf3bb5d
Best regards,
On Mon 2023-08-07 08:36:28, Nathan Chancellor wrote:
A recent change in clang allows it to consider more expressions as compile time constants, which causes it to point out an implicit conversion in the scanf tests:
lib/test_scanf.c:661:2: warning: implicit conversion from 'int' to 'unsigned char' changes value from -168 to 88 [-Wconstant-conversion] 661 | test_number_prefix(unsigned char, "0xA7", "%2hhx%hhx", 0, 0xa7, 2, check_uchar); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/test_scanf.c:609:29: note: expanded from macro 'test_number_prefix' 609 | T result[2] = {~expect[0], ~expect[1]}; \ | ~ ^~~~~~~~~~ 1 warning generated.
The result of the bitwise negation is the type of the operand after going through the integer promotion rules, so this truncation is expected but harmless, as the initial values in the result array get overwritten by _test() anyways. Add an explicit cast to the expected type in test_number_prefix() to silence the warning. There is no functional change, as all the tests still pass with GCC 13.1.0 and clang 18.0.0.
Cc: stable@vger.kernel.org Closes: https://github.com/ClangBuiltLinux/linux/issues/1899
"Closes:" is not a valid tag. It was proposed and rejected in the end. I replaced it with "Link:" as suggested by ./scripts/checkpatch.pl/
Link: https://github.com/llvm/llvm-project/commit/610ec954e1f81c0e8fcadedcd25afe64... Suggested-by: Nick Desaulniers ndesaulniers@google.com Signed-off-by: Nathan Chancellor nathan@kernel.org
Reviewed-by: Petr Mladek pmladek@suse.com
The patch has been pushed into printk/linux.git, branch for-6.6.
Best Regards, Petr
Hi Petr,
On Wed, Aug 16, 2023 at 01:01:46PM +0200, Petr Mladek wrote:
On Mon 2023-08-07 08:36:28, Nathan Chancellor wrote:
A recent change in clang allows it to consider more expressions as compile time constants, which causes it to point out an implicit conversion in the scanf tests:
lib/test_scanf.c:661:2: warning: implicit conversion from 'int' to 'unsigned char' changes value from -168 to 88 [-Wconstant-conversion] 661 | test_number_prefix(unsigned char, "0xA7", "%2hhx%hhx", 0, 0xa7, 2, check_uchar); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/test_scanf.c:609:29: note: expanded from macro 'test_number_prefix' 609 | T result[2] = {~expect[0], ~expect[1]}; \ | ~ ^~~~~~~~~~ 1 warning generated.
The result of the bitwise negation is the type of the operand after going through the integer promotion rules, so this truncation is expected but harmless, as the initial values in the result array get overwritten by _test() anyways. Add an explicit cast to the expected type in test_number_prefix() to silence the warning. There is no functional change, as all the tests still pass with GCC 13.1.0 and clang 18.0.0.
Cc: stable@vger.kernel.org Closes: https://github.com/ClangBuiltLinux/linux/issues/1899
"Closes:" is not a valid tag. It was proposed and rejected in the end. I replaced it with "Link:" as suggested by ./scripts/checkpatch.pl/
I don't really care about "Closes:" vs. "Link:", either is fine with me, but checkpatch.pl did not warn me about it and I still see commit 44c31888098a ("checkpatch: allow Closes tags with links") in mainline and -next that explicitly allows this (and even requires Closes: instead of Link: when following Reported-by:).
Link: https://github.com/llvm/llvm-project/commit/610ec954e1f81c0e8fcadedcd25afe64... Suggested-by: Nick Desaulniers ndesaulniers@google.com Signed-off-by: Nathan Chancellor nathan@kernel.org
Reviewed-by: Petr Mladek pmladek@suse.com
The patch has been pushed into printk/linux.git, branch for-6.6.
Thanks a lot for the review and acceptance!
Cheers, Nathan
On Wed 2023-08-16 07:01:12, Nathan Chancellor wrote:
Hi Petr,
On Wed, Aug 16, 2023 at 01:01:46PM +0200, Petr Mladek wrote:
On Mon 2023-08-07 08:36:28, Nathan Chancellor wrote:
A recent change in clang allows it to consider more expressions as compile time constants, which causes it to point out an implicit conversion in the scanf tests:
lib/test_scanf.c:661:2: warning: implicit conversion from 'int' to 'unsigned char' changes value from -168 to 88 [-Wconstant-conversion] 661 | test_number_prefix(unsigned char, "0xA7", "%2hhx%hhx", 0, 0xa7, 2, check_uchar); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib/test_scanf.c:609:29: note: expanded from macro 'test_number_prefix' 609 | T result[2] = {~expect[0], ~expect[1]}; \ | ~ ^~~~~~~~~~ 1 warning generated.
The result of the bitwise negation is the type of the operand after going through the integer promotion rules, so this truncation is expected but harmless, as the initial values in the result array get overwritten by _test() anyways. Add an explicit cast to the expected type in test_number_prefix() to silence the warning. There is no functional change, as all the tests still pass with GCC 13.1.0 and clang 18.0.0.
Cc: stable@vger.kernel.org Closes: https://github.com/ClangBuiltLinux/linux/issues/1899
"Closes:" is not a valid tag. It was proposed and rejected in the end. I replaced it with "Link:" as suggested by ./scripts/checkpatch.pl/
I don't really care about "Closes:" vs. "Link:", either is fine with me, but checkpatch.pl did not warn me about it and I still see commit 44c31888098a ("checkpatch: allow Closes tags with links") in mainline and -next that explicitly allows this (and even requires Closes: instead of Link: when following Reported-by:).
Good to know. It is possible that I mixed this with another tag. I recall that people wanted to add some new tags recently and Linus was strongly against it. Unfortunately, I can't find the discussion now.
It seems that the Closes: tag is acceptable. But Linus is still going to "put my foot down" when it gets misused, see https://lore.kernel.org/linux-doc/CAHk-=wh0v1EeDV3v8TzK81nDC40=XuTdY2MCr0xy3...
I called the checkpatch.pl from printk/linux.git in a branch based on 6.4. It did not have the commit 44c31888098a ("checkpatch: allow Closes tags with links").
If you do not mind, I'll keep the "Link:" tag to avoid rebase in the for-6.6 branch in printk/linug.git.
The patch has been pushed into printk/linux.git, branch for-6.6.
Thanks a lot for the review and acceptance!
You are welcome.
Best Regards, Petr
linux-stable-mirror@lists.linaro.org
-
Nathan Chancellor -
Petr Mladek