Re: [LTP] [PATCH v1] execl(), execlp() and execle() require proper termination of argument list

Richard Palethorpe
2022-11-29T14:33:30Z

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> On 28. 11. 22 12:11, Petr Vorel wrote:
>> Hi Michael,
>> sorry to bother you, could you please comment our discussion about
>> execl{,e,p}()
>> termination of argument list being NULL vs. (char *)NULL vs. (void*)0?
>> Martin reported [2] that man page suggests (char*)NULL, his view of
>> reason [3]:
>> NULL may be defined as simple integer 0. When int is 32bit and pointers
>> 64bit, this will cause trouble in variadic functions such as execlp().
>> Cyril pointed out [4]: NULL is required to be 0 cast to void* in
>> POSIX. [5]
>> Therefore what should be really used?
>> Kind regards,
>> Petr
>> [2]
>> https://lore.kernel.org/ltp/8587b908-a035-a96a-7233-2863b7bc30ca@suse.cz/
>> [3] https://lore.kernel.org/ltp/af63ed9a-7108-fd19-fe2c-4b56be85d068@suse.cz/
>> [4] https://lore.kernel.org/ltp/Y4DSmk7uY9zUUQsV@yuki/
>> [5] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stddef.h.html
>
> Cyril is correct that we don't need this fix as long as we use C99 or
> later with POSIX-compliant build system. The explicit type cast is
> required only in C++ where there's no explicit conversion from void*
> to other pointer types and therefore NULL must be defined as integer
> instead of void* pointer constant.
>
> Then again, pedantically following the docs won't break anything either.
>
> Acked-by: Martin Doucha <mdoucha@suse.cz>

I think the docs are wrong here, they should probably specify where it
is needed to cast to (char *). We are (now) using a superset of c99 so
we are not effected.

The issue I see is that this is yet another thing to remember to enforce
and for no apparent benefit. OTOH if there is some compiler/analyser
flag which creates a warning then I would not be against adding that.

--
Thank you,
Richard.