Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Several updates - see individual commits #96

Merged
merged 12 commits into from
Oct 26, 2024
Merged

Several updates - see individual commits #96

merged 12 commits into from
Oct 26, 2024

Conversation

fhgwright
Copy link
Contributor

Notably:

  • The darwin_c tests are now included in the automatic tests.
  • Works around Apple's broken versioning in the macOS 15 SDK.
  • Implements scandir() compatibility feature.
  • Fixes dprintf() bug and implements vdprintf().

Tested on:

Mac OS X 10.4.11 8S165, PPC, Xcode 2.5 8M2558
Mac OS X 10.4.11 8S2167, i386, Xcode 2.5 8M2558
Mac OS X 10.5.8 9L31a, PPC, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, i386, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, x86_64, Xcode 3.1.4 9M2809
Mac OS X 10.5.8 9L31a, PPC (i386 Rosetta), Xcode 3.1.4 9M2809
Mac OS X 10.6.8 10K549, i386, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, x86_64, Xcode 3.2.6 10M2518
Mac OS X 10.6.8 10K549, PPC (i386 Rosetta), Xcode 3.2.6 10M2518
Mac OS X 10.7.5 11G63, x86_64, Xcode 4.6.3 4H1503
OS X 10.8.5 12F2560, x86_64, Xcode 5.1.1 5B1008
OS X 10.9.5 13F1911, x86_64, Xcode 6.2 6C131e
OS X 10.10.5 14F2511, x86_64, Xcode 7.2 7C68
OS X 10.11.6 15G22010, x86_64, Xcode 8.1 8B62
macOS 10.12.6 16G2136, x86_64, Xcode 9.2 9C40b
macOS 10.13.6 17G14042, x86_64, Xcode 10.1 10B61
macOS 10.14.6 18G9323, x86_64, Xcode 11.3.1 11C505
macOS 10.15.7 19H15, x86_64, Xcode 12.4 12D4e
macOS 11.7.10 20G1427, x86_64, Xcode 13.2.1 13C100
macOS 11.7.10 20G1427, arm64, Xcode 13.2.1 13C100
macOS 12.7.6 21H1320, x86_64, Xcode 14.2 14C18
macOS 12.7.6 21H1320, arm64, Xcode 14.2 14C18
macOS 13.7 22H123, arm64, Xcode 15.2 15C500b
macOS 14.7 23H124, arm64, Xcode 16.0 16A242d
macOS 15.0 24A335, arm64, Xcode 16.0 16A242d

@fhgwright
Make darwin_c tests automatic.
ac5464f 
These were only manual due to different build requirements.  This
adds a new "xtest" category, which is included in the automatic tests
but meets the requirements for the darwin_c tests.

TESTED:
The darwin_c test are now included in the "test" target.
@fhgwright
Rename MACPORTS_LEGACY_MIN_EARLY_SDK_ALLOWED
18196ea 
It's more consistent with normal naming conventions to use a
leading underscore for the name of this flag.  Thus, the
flag is now _MACPORTS_LEGACY_MIN_EARLY_SDK_ALLOWED.

Since this feature is only recently added and rarely needed, it's
not too likely that this change will break anything.

TESTED:
Updated name has desired effect.
@fhgwright
Fix broken 15.x SDK version
70e5250 
Apple failed to update AvailabilityMacros.h for the 15.0 SDK, so it
appears to be a 14.x SDK.  This adds a workaround for that.

TESTED:
The checksdkversion test now reports correct results for all available
SDKs (10.4-15.0).
@fhgwright
sdkversion.h: add SDK 15.1 undef
8808f1e
@fhgwright
Add scandir() compatibility feature.
d31f7fa 
This adds a wrapper to make the scandir() 'compar' argument and the
provided alphasort() function match the types used in 10.8+, thereby
avoiding warnings, and in some cases errors, due to the pointer type
mismatch.

To avoid breaking code expecting the old behavior, this hack is only
applied when _MACPORTS_LEGACY_COMPATIBLE_SCANDIR is defined nonzero.

Closes: https://trac.macports.org/ticket/70702

TESTED:
Tests pass on all OS versions with this fix.
@fhgwright
Add test for scandir() 'compar' type.
fca9464 
This tests for the pointer-type mismatch issue with the scandir()
'compar' arg.  There are two versions, one verifying the expected old
behavior, and another verifying the fix.

See: https://trac.macports.org/ticket/70702

Also adds alphasort() and scandir() to the darwin_c tests.

TESTED:
Fails or gets warning on <10.8 without fix.  Passes with fix.
@fhgwright
test_scandir: Add version with 32-bit inodes.
a100f90 
TESTED:
Fails or gets warning on <10.8 without fix.  Passes with fix.
@fhgwright
Rewrite dprintf() implementation.
d3019e5 
This is primarily to fix a bug in dprintf() which erroneously closed
the fd after its use, but is also mildly more efficient.

Based on some work by raf <[email protected]>

TESTED:
Passes the updated test, which failed before this fix.
@fhgwright
Rewrite dprintf() test.
40ecf77 
This performs a much more thorough test of dprintf(), including
verifying that it does not close the underlying fd.

TESTED:
Fails when the old close bug is present, passes otherwise.
@fhgwright
Add vdprintf() support.
e376493 
Also rewrites dprintf() to be a wrapper around vdprintf(), rather than
duplicating the "guts".

Based on some work by raf <[email protected]>

TESTED:
Passes updated test.
@fhgwright
Add vdprintf() tests to dprintf() tests.
30ca4ff 
Also adds vdprintf() to darwin_c tests.

TESTED:
Fails without the new vdprintf() addition; passes with it.
@fhgwright
[v]dprintf(): Fix unwanted lock release.
cacf14c 
This uses a kludgy and nonportable method to avoid the ill effects of
fclose() on locks (see the comment).  It should be reliable for all
relevant OSX versions, but it can be reverted if it causes trouble.

TESTED:
Passes tests, which would fail if the close() disable didn't work.
@fhgwright
Copy link
Contributor Author

@mascguy
Here's your manual notification. :-)
Part of this supersedes #95, as previously discussed there. It should be closed.

@macportsraf
I came up with a way to avoid the dup() and the related lock problem. See the big comment.

@fhgwright
Copy link
Contributor Author

@mascguy
Ping.

@fhgwright
Copy link
Contributor Author

@mascguy
Ah, you're alive. :-)

I have another bunch of changes coming, but my intent is to make this round into a -devel version first. The next round is mostly about cleaning up after SDK 15, which has really proved to be the gift that keeps on giving (and not in a good way).

@fhgwright
Copy link
Contributor Author

@mascguy
If you don't have time to review this, maybe there's someone else who does. This is over three weeks old now, and my post to the ML didn't accomplish anything other than getting the obsolete PR closed.

@macportsraf
Copy link

@fhwright I don't see an explanation why a local buffer is used in dprintf. Could the reason be documented? Or have I missed it? I would have thought that fdopen would create a buffer. Is there a problem with using that one?

@fhgwright
Copy link
Contributor Author

@macportsraf

@fhwright I don't see an explanation why a local buffer is used in dprintf. Could the reason be documented? Or have I missed it? I would have thought that fdopen would create a buffer. Is there a problem with using that one?

It's better to get the free allocation on the stack than to go through malloc() and free(), which would also require additional internal knowledge of FILE to find the buffer address. Note that Apple's code does the same thing.

Apple's code also uses the stack for the FILE, but that requires way more internal knowledge than this code is assuming. It's trying to use the bare minimum of internal knowledge necessary to accomplish the goal, which is to avoid both closing the underlying fd and releasing any locks that may be in effect.

@fhgwright
Copy link
Contributor Author

@macportsraf
Actually, upon reflection, I think the fclose() would handle the free(). But it still wouldn't make sense to incur the malloc()/free() overhead just to avoid a modest-sized buffer on the stack. The reason one can't use free() on a FILE is that FILE uses a special allocator just to avoid the malloc()/free() overhead.

mascguy
mascguy approved these changes Oct 26, 2024
View reviewed changes
Copy link
Member

@mascguy mascguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. LGTM, great work as always folks!

@mascguy mascguy merged commit 4b3dd36 into macports:master Oct 26, 2024
mascguy added a commit to macports/macports-ports that referenced this pull request Oct 26, 2024
@mascguy
legacy-support-devel: update to latest master
24ce680 
- The darwin_c tests are now included in the automatic tests
- Works around Apple's broken versioning in the macOS 15 SDK
- Implements scandir() compatibility feature
- Fixes dprintf() bug and implements vdprintf()

See: macports/macports-legacy-support#96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mascguy mascguy mascguy approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fhgwright @macportsraf @mascguy