linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix()
@ 2023-08-03 18:14 Nathan Chancellor
  2023-08-04  4:25 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2023-08-03 18:14 UTC (permalink / raw)
  To: pmladek, rostedt
  Cc: senozhatsky, andriy.shevchenko, linux, ndesaulniers, trix,
	linux-kernel, llvm, patches, Nathan Chancellor

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.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1899
Link: https://github.com/llvm/llvm-project/commit/610ec954e1f81c0e8fcadedcd25afe643f5a094e
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 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..10d557b3738f 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,
-- 
Nathan Chancellor <nathan@kernel.org>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix()
  2023-08-03 18:14 [PATCH] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix() Nathan Chancellor
@ 2023-08-04  4:25 ` Andy Shevchenko
  2023-08-04 14:48   ` Nathan Chancellor
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2023-08-04  4:25 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: pmladek, rostedt, senozhatsky, linux, ndesaulniers, trix,
	linux-kernel, llvm, patches

On Thu, Aug 03, 2023 at 11:14:42AM -0700, 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.

>  do {										\
>  	const T expect[2] = { expect0, expect1 };				\
> -	T result[2] = {~expect[0], ~expect[1]};					\
> +	T result[2] = {(T)~expect[0], (T)~expect[1]};				\

Can we add spaces as above, while at it?

	T result[2] = { (T)~expect[0], (T)~expect[1] };				\

>  	_test(fn, &expect, str, scan_fmt, n_args, &result[0], &result[1]);	\
>  } while (0)

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix()
  2023-08-04  4:25 ` Andy Shevchenko
@ 2023-08-04 14:48   ` Nathan Chancellor
  0 siblings, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2023-08-04 14:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: pmladek, rostedt, senozhatsky, linux, ndesaulniers, trix,
	linux-kernel, llvm, patches

On Fri, Aug 04, 2023 at 07:25:00AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 03, 2023 at 11:14:42AM -0700, 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.
> 
> >  do {										\
> >  	const T expect[2] = { expect0, expect1 };				\
> > -	T result[2] = {~expect[0], ~expect[1]};					\
> > +	T result[2] = {(T)~expect[0], (T)~expect[1]};				\
> 
> Can we add spaces as above, while at it?
> 
> 	T result[2] = { (T)~expect[0], (T)~expect[1] };				\

Sure. I can send a v2 on Monday to give folks a chance to chime in with
other comments.

> >  	_test(fn, &expect, str, scan_fmt, n_args, &result[0], &result[1]);	\
> >  } while (0)
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-04 14:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 18:14 [PATCH] lib: test_scanf: Add explicit type cast to result initialization in test_number_prefix() Nathan Chancellor
2023-08-04  4:25 ` Andy Shevchenko
2023-08-04 14:48   ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).