This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Limit catopen usage to unix-like OSes
ClosedPublic

Authored by bcraig on Jan 27 2016, 10:37 AM.

Details

Summary

Operating systems that are not unix-like are unlikely to have access to catopen. Instead of black-listing each one, we now filter out all non-unix operating systems first. We then exclude the unix-like operating systems that don't have catopen. _WIN32 counts as a unix-like operating system because of cygwin.

Diff Detail

Event Timeline

bcraig updated this revision to Diff 46150.Jan 27 2016, 10:37 AM
bcraig retitled this revision from to [libcxx] Limit catopen usage to unix-like OSes.
bcraig updated this object.
bcraig added reviewers: mclow.lists, ed, jfb, EricWF, jroelofs.
bcraig added a subscriber: cfe-commits.
ed edited edge metadata.Jan 27 2016, 1:13 PM

I'm usually not a big fan of using definitions such as __unix__, because it is pretty vague and defined inconsistently. That said, if you're going to use this definition, then there is some good news: CloudABI doesn't define it, meaning that you could just remove it from this list.

bcraig updated this revision to Diff 46291.Jan 28 2016, 10:10 AM
bcraig edited edge metadata.

CloudABI doesn't define unix, so stop blacklisting it explicitly and let the whitelist filter it out.

mclow.lists edited edge metadata.Feb 9 2016, 7:48 AM

I don't have a problem with this change (assuming that it is correct, and it looks correct), but I think it should stay in __config, with all the other configuration tests. (There are other configuration tests in the .cpp files, but users never see those).

FWIW, looking at locale, I think that code can be significantly cleaned up.

bcraig updated this revision to Diff 47328.Feb 9 2016, 9:00 AM
bcraig updated this object.
bcraig edited edge metadata.

Moved the CATOPEN config check back to __config. Saving a larger locale refactor for later (but oh boy is it coming).

mclow.lists accepted this revision.Feb 9 2016, 6:11 PM
mclow.lists edited edge metadata.

This looks good to me.

Saving a larger locale refactor for later (but oh boy is it coming).

Looking forward to it.

This revision is now accepted and ready to land.Feb 9 2016, 6:11 PM
bcraig closed this revision.Feb 10 2016, 5:51 AM