Re: [LTP] [PATCH v3 1/8] syscalls/mknod01: Fix checkpatch.pl warnings

["Ricardo B. Marlière via ltp" <ltp@lists.linux.it>]

Hello Petr!

On Tue Apr 22, 2025 at 7:52 AM -03, Petr Vorel wrote:
> Hi Ricardo,
>
>> Fix the following warnings:
>
>> 	Alignment should match open parenthesis
>> 	Please don't use multiple blank lines
>
> I'm not sure how you invoke checkpatch.pl.
> Could you please use LTP embedded version via:
>
> $ make check-mknod01
>
> Or, to see what it does use V=1
>
> $ make check-mknod01 V=1
>
>> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
>> ---
>>  testcases/kernel/syscalls/mknod/mknod01.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>
>> diff --git a/testcases/kernel/syscalls/mknod/mknod01.c b/testcases/kernel/syscalls/mknod/mknod01.c
>> index fe0a1cfa6a473f9c2b2a55493f830f13b86560a8..39e402ee412de39c7929dcae65fdf3e302de9b45 100644
>> --- a/testcases/kernel/syscalls/mknod/mknod01.c
>> +++ b/testcases/kernel/syscalls/mknod/mknod01.c
>> @@ -26,7 +26,6 @@ static int tcases[] = {
>>  	S_IFREG | 06700,
>>  };
>
>> -
>
> This is not reported by our checkpatch.pl and it would be useful. I'm not sure
> if you want to bother to look which from these --ignore
> CONST_STRUCT,VOLATILE,SPLIT_STRING,FILE_PATH_CHANGES should be removed (or which
> one is missing, maybe our checkpatch.pl is outdated).
>

Thanks for pointing this out, I got these warnings by passing `--strict`
option to checkpatch.pl. Also, I removed FILE_PATH_CHANGES from that
list but I don't remember now why.

>>  static void run(unsigned int i)
>>  {
>>  	dev_t dev = 0;
>> @@ -35,8 +34,8 @@ static void run(unsigned int i)
>>  		dev = makedev(1, 3);
>
>>  	TST_EXP_PASS(mknod(PATH, tcases[i], dev),
>> -				"mknod(PATH, %o, %ld)",
>> -				tcases[i], dev);
>> +		     "mknod(PATH, %o, %ld)",
>> +		     tcases[i], dev);
>
> I'm not sure, if this is worth to fix (our checkpatch.pl setup does not report it).
>

Indeed, its a purely aesthetic patch.

> But what would be worth to fix is 32 bit compatibility error on dev_t dev:
>
> $ PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig CFLAGS=-m32 LDFLAGS=-m32 ./configure # see INSTALL
> ...
>
> mknod01.c:37:22: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 6 has type ‘dev_t’ {aka ‘long long unsigned int’} [-Wformat=]
>    37 |                      "mknod(PATH, %o, %ld)",
>       |                      ^~~~~~~~~~~~~~~~~~~~~~
> ../../../../include/tst_test_macros.h:42:32: note: in definition of macro ‘TST_FMT_’
>    42 | #define TST_FMT_(FMT, _1, ...) FMT, ##__VA_ARGS__
>       |                                ^~~
> ../../../../include/tst_test_macros.h:46:26: note: in expansion of macro ‘TST_2_’
>    46 |                 TST_FMT_(TST_2_(dummy, ##__VA_ARGS__, SCALL) FMT, __VA_ARGS__))
>       |                          ^~~~~~
> ../../../../include/tst_test_macros.h:210:25: note: in expansion of macro ‘TST_MSG_’
>   210 |                         TST_MSG_(TPASS, " passed", #SCALL, ##__VA_ARGS__);     \
>       |                         ^~~~~~~~
> mknod01.c:36:9: note: in expansion of macro ‘TST_EXP_PASS’
>    36 |         TST_EXP_PASS(mknod(PATH, tcases[i], dev),
>       |         ^~~~~~~~~~~~
> mknod01.c:37:41: note: format string is defined here
>    37 |                      "mknod(PATH, %o, %ld)",
>       |                                       ~~^
>       |                                         |
>       |                                         long int
>       |                                       %lld
>
> (It should be solved by PRIuMAX and casting to (uintmax_t), but we usually just
> cast to (long long) and use just %lld. Whatever you prefer.
>

Nice, I'll add that :)

Thank you,
-	Ricardo.


> Kind regards,
> Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp