Skip to content

Fix multi-session skeleton#366

Merged
superg merged 2 commits intosuperg:mainfrom
Deterous:main
Mar 30, 2026
Merged

Fix multi-session skeleton#366
superg merged 2 commits intosuperg:mainfrom
Deterous:main

Conversation

@Deterous
Copy link
Copy Markdown
Contributor

@Deterous Deterous commented Mar 30, 2026

For multi-session skeletons, the LBA does not start at 0 and the detected files don't get erased.
This fix adds the LBA of the first sector to the inside-contents check to account for this.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed sector offset alignment in ISO9660 skeleton creation to correctly handle file system area mapping.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fb33ce5d-a152-4dca-8d05-f403e1f569f3

📥 Commits

Reviewing files that changed from the base of the PR and between 101f4a6 and 964d971.

📒 Files selected for processing (1)
  • skeleton.ixx

📝 Walkthrough

Walkthrough

The change adjusts sector indexing in skeleton creation logic to align sector numbering with the ISO9660 area map's starting LBA offset by adding area_map[0].lba to the sector index when checking membership.

Changes

Cohort / File(s) Summary
Sector offset alignment
skeleton.ixx
Modified membership check during sector erasure to account for area map's starting LBA offset by adjusting sector index from s to s + area_map[0].lba.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A sector shifts, its offset found,
Where LBA marks the ISO ground,
No longer lost in numbered space,
Each block now knows its proper place! 📍

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix multi-session skeleton' directly addresses the main change: adjusting sector indexing for multi-session skeleton creation by accounting for non-zero starting LBA.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@superg
Copy link
Copy Markdown
Owner

superg commented Mar 30, 2026

Good catch!

@superg superg merged commit 2accb81 into superg:main Mar 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants