Cleanup PosixSource.h
ClosedPublic

Authored by angerman on Oct 7 2016, 3:25 AM.

Details

Summary

When trying to build arm64-apple-iso, the build fell over
strdup, as the arm64-apple-ios build did not fall into darwin_HOST_OS,
and would need ios_HOST_OS.

This diff tries to clean up PosixSource.h, instead of layering another
define on top.

As we use strnlen in sources that include PosixSource.h, and strnlen
is defined in POSIX.1-2008, the _POSIX_C_SOURCE and _XOPEN_SOURCE
are increased accordingly.

Furthermore the _DARWIN_C_SOURCE (required for u_char, etc. used in
sysctl.h) define is moved into OSThreads.h alongside a similar ifdef
for freebsd.

Test Plan

Build on all supported platforms.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
angerman updated this revision to Diff 8922.Oct 7 2016, 3:25 AM
angerman retitled this revision from to Cleanup PosixSource.h.
angerman updated this object.
angerman edited the test plan for this revision. (Show Details)
angerman added reviewers: bgamari, kgardas.
angerman updated the Trac tickets for this revision.
angerman added a subscriber: thomie.
angerman updated this revision to Diff 8924.Oct 7 2016, 3:30 AM
angerman edited edge metadata.

rebase onto master

Note: this is essentially the revival of D2375, hopefully not breaking Oracle Solaris this time.

simonmar accepted this revision.Oct 7 2016, 4:10 AM
simonmar edited edge metadata.

gogogogogo

This revision is now accepted and ready to land.Oct 7 2016, 4:10 AM
erikd added a subscriber: hvr.Oct 7 2016, 4:14 AM
erikd added inline comments.
rts/PosixSource.h
28

I can't remember if we still need that! @bgamari @hvr ?

erikd accepted this revision.Oct 7 2016, 4:21 AM
erikd edited edge metadata.
erikd added inline comments.
rts/PosixSource.h
28

No, I don't think we do. I *think* is needed in includes/stg/Types.h and that has its own #define.

Just to add: we use strnlen here:

rts/RtsUtils.c
112-/* borrowed from the MUSL libc project */
113-char *stgStrndup(const char *s, size_t n)
114-{
115:    size_t l = strnlen(s, n);
116-    char *d = stgMallocBytes(l+1, "stgStrndup");
117-    if (!d) return NULL;
118-    memcpy(d, s, l);
119-    d[l] = 0;
120-    return d;
121-}

which was introduced in d1ce35d by @bgamari, in D2125.

kgardas edited edge metadata.Oct 7 2016, 11:25 AM

Works on Solaris 11.2. Tested on i386 platform, but since this is hardware platform independent issue I would not expect any issue on amd64. Thanks for the patch! Karel

kgardas accepted this revision.Oct 7 2016, 11:25 AM
kgardas edited edge metadata.
bgamari accepted this revision.Oct 7 2016, 8:47 PM
bgamari edited edge metadata.
bgamari added a subscriber: Phyx.

I wonder why Harbormaster didn't build this on OS X.

Otherwise looks good to me.

rts/PosixSource.h
28

Oh dear, I'm afraid I don't remember We should really add a comment if it's still needed. @Phyx?

Ahh, never mind. Because it's configured not to build diffs; naturally.

Just to add: we use strnlen here:

...

which was introduced in d1ce35d by @bgamari, in D2125.

Indeed; moreover, if we had _POSIX_C_SOURCE >= 200809 I would have just used strndup there instead of redefining it.

Phyx added inline comments.Oct 7 2016, 9:10 PM
rts/PosixSource.h
28

It shouldn't be, the _mingw headers would define it due to the _POSIX_C_SOURCE

/* We are activating __USE_MINGW_ANSI_STDIO for various define indicators.
   Note that we enable it also for _GNU_SOURCE in C++, but not for C case. */
#if (defined (_POSIX) || defined (_POSIX_SOURCE) || defined (_POSIX_C_SOURCE) \
     || defined (_ISOC99_SOURCE) \
     || defined (_XOPEN_SOURCE) || defined (_XOPEN_SOURCE_EXTENDED) \
     || (defined (_GNU_SOURCE) && defined (__cplusplus)) \
     || defined (_SVID_SOURCE)) \
    && !defined(__USE_MINGW_ANSI_STDIO)
/* Enable __USE_MINGW_ANSI_STDIO if _POSIX defined
 * and If user did _not_ specify it explicitly... */
#  define __USE_MINGW_ANSI_STDIO                        1
#endif

A simple testcase can be added for this, just any test that uses a C99 printf modifier.

which was introduced in d1ce35d by @bgamari, in D2125.

Indeed; moreover, if we had _POSIX_C_SOURCE >= 200809 I would have just used strndup there instead of redefining it.

@bgamari Does this imply that d1ce35d would be unnecessary with the lifting to IEEE Std 1003.1-2008 (XPG7)?

This revision was automatically updated to reflect the committed changes.