Re: [PATCH v2] selftests/ptrace/get_syscall_info: fix for MIPS n32

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/29/25 08:02, Maciej W. Rozycki wrote:
> On Sat, 29 Mar 2025, Dmitry V. Levin wrote:
> 
>>>> +#if defined(_MIPS_SIM) && _MIPS_SIM == _MIPS_SIM_NABI32
>>>> +/*
>>>> + * MIPS N32 is the only architecture where __kernel_ulong_t
>>>> + * does not match the bitness of syscall arguments.
>>>> + */
>>>> +typedef unsigned long long kernel_ulong_t;
>>>> +#else
>>>> +typedef __kernel_ulong_t kernel_ulong_t;
>>>> +#endif
>>>> +
>>>
>>> What's the reason for adding these typedefs? checkpatch should
>>> have warned you about adding new typedefs.
>>>
>>> Also this introduces kernel_ulong_t in user-space test code.
>>> Something to avoid.
>>
>> There has to be a new type for this test, and the natural way to do this
>> is to use typedef.  The alternative would be to #define kernel_ulong_t
>> which is ugly.  By the way, there are quite a few typedefs in selftests,
>> and there seems to be given no rationale why adding new types in selftests
>> is a bad idea.
> 

It causes problems down the road for maintenance. I would rather not
see these types of kernel typedefs added to user-space.

>   FWIW I agree, and I fail to see a reason why this would be a problem in a
> standalone test program where the typedef does not propagate anywhere.
> 
>   The only potential issue I can identify would be a namespace clash, so
> perhaps the new type could have a name prefix specific to the test, but it
> doesn't appear to me a widespread practice across our selftests and then
> `kernel_' ought to be pretty safe against ISO C or POSIX, so perhaps let's
> leave the things alone?
> 

Can't this be solved with ifdef?

thanks,
-- Shuah