BACKENDS: MACOS: Fix various small warnings in macosx-audiocd.cpp#6477
Merged
dwatteau merged 2 commits intoscummvm:masterfrom Apr 16, 2025
Merged
BACKENDS: MACOS: Fix various small warnings in macosx-audiocd.cpp#6477dwatteau merged 2 commits intoscummvm:masterfrom
dwatteau merged 2 commits intoscummvm:masterfrom
Conversation
Allocate the `struct statfs` buffer on the heap instead of the stack, fixing a -Wframe-larger-than warning. Fix a -Wsign-compare warning with the GCC 32-bit builds. Add a couple more headers for getfsstat(), as given by its manual page (although it builds just fine without them).
criezy
reviewed
Mar 11, 2025
criezy
reviewed
Mar 12, 2025
…ant buffer For the statfs buffer used by detectAllDrives(), only allocate what is needed, instead of an arbitrary value. Calling getfsstat() a first time with a NULL pointer will return the number of mounted file systems. We can then use that a allocate a buffer at the right size for the next getfsstat() call. It looks like we can then reasonably expect MNT_NOWAIT to be enough for the second call that immediately follows... Idea from criezy.
88d82b6 to
74b34a2
Compare
Contributor
Author
|
@criezy I've made some adjustments from your last review. FWIW, alternatives to
the last two APIs maybe suit this code better. Maybe they'd make detecting an audio CD drive faster, too, but I haven't tested that. I don't think we'd gain much by switching to some other API anyway. The audiocd code is probably less and less used today, so I'd rather not diverge too much from something that didn't cause much bug reports in the 12 years it's been there. OK for you? Thanks! |
Contributor
Author
|
Merging, since there was no objection. Thanks! |
This file contains hidden or 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
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.
This is for
macosx-audiocd.cpp(backend code).-Wframe-larger-thanwarning (macOS 13.7.4, Intel x64, Apple Clang 14), by allocating thestatfsbuffer on the heap instead of the stack-Wsign-comparewarning when comparing withUINT_MAX(macOS 10.4.11, ppc32, GCC 7.5.0), with anunsigned longcast (since we already checktrackID > 0just before it)While there:
getfsstat(), as given by its manual page (although it always built just fine without them).I can split this into multiple commits if you'd like, but I'm not sure it's worth it here.
Tested on macOS 10.4 (ppc32), 13.7 (Intel x64) and 15.3 (aarch64).
@criezy: Does this look good to you? Thanks :)