mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: Colin Cross <ccross@google.com>
Cc: musl@lists.openwall.com, Ismael Luceno <ismael@iodev.co.uk>
Subject: Re: [musl] Re: [PATCH] Define NULL as nullptr when used in C++
Date: Thu, 23 Dec 2021 16:05:22 -0500	[thread overview]
Message-ID: <20211223210521.GR7074@brightrain.aerifal.cx> (raw)
In-Reply-To: <CAMbhsRQ0p-e5DjGrUvPZZmepdeAHfvGQKmMWMiBs+a4BPMA14Q@mail.gmail.com>

On Thu, Dec 23, 2021 at 11:13:01AM -0800, Colin Cross wrote:
> On Sun, Aug 15, 2021 at 05:51:57PM +0200, Ismael Luceno wrote:
> > This should be safer for casting and more compatible with existing code
> > bases that wrongly assume it must be defined as a pointer.
> 
> This seems to meet the C++ spec for NULL [1], but I noticed some
> compatibility issues with code that was previously compiling with
> glibc.
> 
> I've found multiple places that used reinterpret_cast<int*>(NULL),
> which now fail with:
> error: reinterpret_cast from 'nullptr_t' to 'int *' is not allowed
> According to [2] those should technically be static_cast and not
> reinterpret_cast.

This is an improvement then. reinterpret_cast is very very wrong here
and should produce a compile error. It's the equivalent of writing (in
C):

	*(int **)&(void *){NULL}

instead of

	(int *)NULL

i.e. it's type punning where the author intended a value conversion.

> I've also seen failures with returning NULL from a function with a
> jlong return type:
> error: cannot initialize return object of type 'jlong' (aka 'long')
> with an rvalue of type 'nullptr_t'

This also seems to be working as intended, catching a type error
(although the code was valid in earlier C++ versions).

> glibc uses __null if __GNUG__ is set, ((void*)0) for __cplusplus, or 0 for C.

I think you have cases 2 and 3 reversed. We've intentionally never
done case 1 because it's arguably non-conforming (especially in older
C++ versions) where stringifying the expansion of NULL could observe
that it's not actually an integer constant expression.

The later (C++11) allowance for it to be nullptr offered a way to do
this that is conforming and that catches more programming errors,
which was why folks were able to convince me to finally adopt it even
though I repeatedly rejected the __null thing.

> This also meets the C++ spec for NULL [1], but is an improvement over
> the previous 0L because it can be correctly interpreted as a NULL
> sentinel value by Clang's -Wsentinel warning.
> 
> Ismael, can you give an example of the code that assumes NULL is a
> pointer?  Does it work with __null (assuming you're using a compiler
> that has GNU extensions like __null)?

I don't think the main aim here is to support code that wrongly
assumes NULL has pointer type, but to catch wrong code that's assuming
it doesn't or just accidentally doing something even more wrong.
Avoiding the warning spam compiling GCC itself is kinda nice but it's
a bonus.

> In any case, I'll fix the technically incorrect code I have access to
> so that it works with nullptr.

Sounds like a good plan.

Rich

  reply	other threads:[~2021-12-23 21:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-23 19:13 Colin Cross
2021-12-23 21:05 ` Rich Felker [this message]
2021-12-23 22:01   ` Ismael Luceno
2021-12-24  1:28   ` Rich Felker

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=20211223210521.GR7074@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=ccross@google.com \
    --cc=ismael@iodev.co.uk \
    --cc=musl@lists.openwall.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.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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).