Repository navigation
Fix inverted big-endian extent check in ISO 9660 directory parsing - #98
Merged
Merged
Conversation
ParseDirectory parsed the big-endian extent when the extent location was valid (both values equal) instead of when it was ambiguous, so every directory was parsed again, once per directory level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
I've tagged Det to review this because I'm not 100% sure what that check was supposed to be doing in the first place. |
Member
|
Yes the Up to you whether all that test code is needed. |
mnadareski
approved these changes
Oct 15, 2026
mnadareski
left a comment
mnadareski
left a comment
Member
There was a problem hiding this comment.
The tests do feel a bit overkill, but they definitely prove it's working. The test suite for the readers is lacking, to say the least.
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
3 participants
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.
ParseDirectoryparses the big-endian extent whendr.ExtentLocation.IsValid(both values equal), but the comment says it should do so when the location is ambiguous.ISO9660.Extraction.csandISO9660.Printing.csalready use!IsValidfor this check.With the current condition every directory is parsed a second time together with its subtree, so the reads double with each directory level, and a differing big-endian location is never parsed.
Measured with
ISO9660.Createon 8 Redump images (PS1 to PS5, three PC discs): the parsed model is identical, the reads drop, e.g. Minecraft PS3 Edition from 26.9 MiB to 0.29 MiB, Commandos from 2.3 MiB to 0.18 MiB.Two new
ParseDirectorytests fail without the change and pass with it.Prepared with AI assistance (Claude Opus 5.5) and reviewed before submission.