[patch] Let stddef.h redefine NULL if __need_NULL is set

[Richard Smith]

On Mon, Apr 28, 2014 at 7:11 PM, Nico Weber <thakis at chromium.org> wrote:

> On Mon, Apr 28, 2014 at 5:45 PM, Nico Weber <thakis at chromium.org> wrote:
> > On Mon, Apr 28, 2014 at 5:39 PM, Richard Smith <richard at metafoo.co.uk>
> > wrote:
> >>
> >> As far as I can see, if one of the __need_FOO macros is defined, glibc
> >> expects <stddef.h> to provide *only* FOO, and not any of the other
> pieces of
> >> <stddef.h>, so I don't think this patch is entirely right. It's also not
> >> complete -- we should also handle FOO in the set {wchar_t, size_t,
> >> ptrdiff_t, wint_t}.
> >
> >
> > wint_t is already handled, right below the lines my patch touches (and in
> > the same way as in my patch). The others don't seem to be needed at the
> > moment, so I'd omit them from now.
>
> Sounds like I misunderstood Richard's reply to this on IRC and he
> thought this was less convincing than I did :-)
>
> For comparison, here are two other possible patches:
>
> * clang-neednull-2.patch changes things so that if __need_NULL is
> defined, stddef.h defines NULL and nothing else. It also changes
> stddef.h so that __need_win_t defines only wint_t and does nothing
> else – this latter part happens to break a test, so the patch also
> updates that test. As far as I can tell from inspection, the test
> wouldn't pass with gcc's stddef.h either, so this improves
> compatibility but it might break existing code.
>
> * clang-neednull-3.patch also adds support for the rest of FOO in the
> set {wchar_t, size_t, ptrdiff_t}.
>
> Let me know which one of the 3 patches (with 1 being the one I
> originally sent) you like best. If it's 3, I'll add a few more tests
> for that.


I strongly prefer patch 3 over 2. (With just patch 2, I worry that some
libc header would say "give me NULL and FOO", and just get NULL.)

Instead of repeating an 'ifdef _STDDEF_H_imp' test, could you define all
the __need_* macros in the _STDDEF_H_imp section? I'd find that slightly
easier to reason about.

>> The intent appears to be to support libc headers such as <stdlib.h>
> (which
> >> provides NULL and size_t, but is *not* allowed to provide any of the
> other
> >> parts of <stddef.h>), and *not* to recover from the <linux/*>, <asm/*>,
> etc.
> >> headers breaking the definition of NULL. I can see no evidence of any
> header
> >> including the broken NULL definition and trying to fix it, only headers
> >> asking for subsets of <stddef.h>.
> >>
> >> So... I'm not opposed to this patch, if it does the right thing, but I
> >> don't think it's a (complete) solution to the problem of getting a bad
> >> definition of NULL from <linux/stddef.h>.
> >>
> >>
> >> On Mon, Apr 28, 2014 at 1:45 PM, Chandler Carruth <chandlerc at google.com
> >
> >> wrote:
> >>>
> >>>
> >>> On Mon, Apr 28, 2014 at 1:16 PM, Nico Weber <thakis at chromium.org>
> wrote:
> >>>>
> >>>> On Mon, Apr 28, 2014 at 1:05 PM, Reid Kleckner <rnk at google.com>
> wrote:
> >>>>>
> >>>>> Even if we commit this workaround, can we report this as a bug to
> >>>>> upstream Linux?
> >>>>
> >>>>
> >>>> As mentioned above, I'm guessing Linux probably doesn't want to depend
> >>>> on C standard headers, so they wouldn't see this as a bug in Linux.
> >>>
> >>>
> >>> Just FYI, there is a more subtle distinction here.
> >>>
> >>> Linux probably wants to not depend on a C standard library. But
> stddef.h
> >>> and the definition of NULL is actually available even in a
> *freestanding*
> >>> implementation of C which has no standard library. It's required to be
> >>> provided by the compiler. As such, I actually think Linux would be OK
> with
> >>> including stddef.h from a technical perspective. Any barrier here
> would be
> >>> historical or political.
> >>>
> >>> That said, either historical or political barriers would be barriers
> all
> >>> the same, and more pressingly we can't retroactively change all of the
> >>> existing linux kernel headers and glibcs deployed around the world and
> >>> trying to use Clang. So suggesting *only* changing either Linux or
> glibc is
> >>> a non-starter. We need to both change Clang to work around this, and
> (where
> >>> we can) suggest to the upstream communities a more clean solution.
> >>>
> >>> _______________________________________________
> >>> cfe-commits mailing list
> >>> cfe-commits at cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140428/06167f10/attachment.html>