All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Palethorpe <rpalethorpe@suse.de>
To: Martin Doucha <mdoucha@suse.cz>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>,
	ltp@lists.linux.it, David Hildenbrand <david@redhat.com>
Subject: Re: [LTP] [PATCH v1] execl(), execlp() and execle() require proper termination of argument list
Date: Tue, 29 Nov 2022 14:22:22 +0000	[thread overview]
Message-ID: <87wn7dsudx.fsf@suse.de> (raw)
In-Reply-To: <c43283e1-e969-46e0-f933-a18c2a6428f5@suse.cz>

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.

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

  reply	other threads:[~2022-11-29 14:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-25 12:25 [LTP] [PATCH v1] execl(), execlp() and execle() require proper termination of argument list David Hildenbrand
2022-11-25 14:06 ` Petr Vorel
2022-11-28  9:45   ` David Hildenbrand
2022-11-28 11:11     ` Petr Vorel
2022-11-28 12:00       ` Martin Doucha
2022-11-29 14:22         ` Richard Palethorpe [this message]
2022-12-05  9:22           ` Richard Palethorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wn7dsudx.fsf@suse.de \
    --to=rpalethorpe@suse.de \
    --cc=david@redhat.com \
    --cc=ltp@lists.linux.it \
    --cc=mdoucha@suse.cz \
    --cc=mtk.manpages@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.