Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 23 Dec 2021 16:05:22 -0500
From: Rich Felker <dalias@...c.org>
To: Colin Cross <ccross@...gle.com>
Cc: musl@...ts.openwall.com, Ismael Luceno <ismael@...ev.co.uk>
Subject: Re: Re: [PATCH] Define NULL as nullptr when used in C++

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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.