-
-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanups and a few minor bugfixes #91
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The dirfuncs_compat test uses the dirfuncs test source, and hence needs a dependency on it. TESTED: Running "make test_dirfuncs_compat" after "touch test/test_dirfuncs.c" now correctly rebuilds the test.
This fixes three issues with specifying flag variables to the Makefile: 1) It attempted to insert ARCHFLAGS into CC and CXX, but it was using ?=, which doesn't work for predefined variables. 2) It never incorporated ARCHFLAGS into LDFLAGS. 3) It provided some defaults in CFLAGS and CXXFLAGS, but any setting of these variables removed those defaults. The defaults are now in XCFLAGS and XCXXFLAGS, which are added to C[XX]FLAGS, along with ARCHFLAGS, so they normally are kept. If removing the defaults is desired, then XC[XX]FLAGS can be overridden. Note that the port build procedure uses FORCE_ARCH, not ARCHFLAGS. Closes: https://trac.macports.org/ticket/69782 TESTED: ARCHFLAGS is now respected, and setting CFLAGS doesn't remove defaults.
TESTED: Builds and passes tests on all platforms.
TESTED: Builds and passes tests on all platforms, including mismatched SDK cases.
TESTED: Builds and passes tests on all platforms.
TESTED: Tests pass normally, and fail with a manually induced error.
This adds a new composite definition to simplify conditionals, and updates the feature macros accordingly. See the comments for more details. TESTED: Tests pass on all platforms.
TESTED: Tests pass on all platforms.
Includes comments explaining the new feature-flag setup. TESTED: Tests pass on all platforms, including mismatched SDK cases.
This splits the feature flag for stpncpy() into separate flags for the target OS version and the SDK version (the first of many such cases). For the library, this just renames the feature flag. For the header, it's complicated by the added feature of fixing the possible incompatibility between 10.7+ SDKs and old compilers. See the comments in string.h. TESTED: Tests pass on all platforms, including mismatched SDK cases.
This makes the proper SDK/lib split for the feature flag. With a proper stdlib.h wrapper, there's no need for a private arc4random.h, nor is there any need to test the feature flag in the test. TESTED: Tests pass on all platforms, including mismatched SDK cases.
This was missing from the README list. There's also no need for the version threshold macro when it uses an ifndef. TESTED: Tests pass on all platforms, though there's no specific test for this feature. That should be added in the future. The change is fairly simple.
This constant was misdefined to be the same value as CLOCK_UPTIME_RAW. In the simple case, it made no difference since both cases are treated identically by the library. But that meant that the library didn't support CLOCK_UPTIME_RAW_APPROX with the correct value, which could occur if the client were built with an SDK that defined it. This can be seen by running test_time.cpp built with an SDK>=10.12 on OS<10.12, though only in the text output, since the test "succeeds" even when the functions don't. TESTED: The aforementioned failing case now works properly.
The implementation of fmemopen() was lacking the usual conditional, so that it was present even in OS versions where the OS provides the function. At best, this was a bit of code bloat. At worst, it might substitute an inferior implementation in later OS versions, rather than getting out of the way. TESTED: Tests pass. The fmemopen() function is now absent on 10.13 builds, while still being present in 10.12 builds.
This allows passing command-line options to tests invoked via the "make" command. TESTED: Works with old checksdkflagvalues, that had -v arg.
mascguy
approved these changes
Jun 30, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the individual commit descriptions.
Tested on:
All tests were run with all compatible SDK versions >= the target OS version. "Header-only" tests were also run with "earlier" SDKs.