Skip to content

Commit 3113760

Browse files
committed
Address CodeRabbit third review round on PR 2013
- Flat view trailing gap: bound by iso->getTD(0).toLBA() (actual loaded image end) instead of PVD_VolumeSpaceSize (logical volume length). - Extract/Replace now persist the sector mode alongside the selection and pass it into CDRIsoFile, matching Hex Edit; gaps/system use RAW, hidden files use their detected M1/M2_FORM1/M2_FORM2 mode. - Replace coroutine: aborts on I/O errors with a printed message instead of silently zero-padding over read/write failures. Also clamps replacement->size() without truncating through uint32_t. - Gap scan: unrecognized sector modes now accumulate into a gap entry (instead of silently dropping bytes) and the scan yields periodically so long runs don't freeze the UI. - readerListDir: the C++ side now strips "." / ".." sentinel entries so Lua callers never see them, regardless of how LuaJIT's ffi.string happens to encode the single-NUL filename. - collectAllEntries (Lua) and collectFlatEntries (GUI) now carry a visited-LBA set to guard against directory cycles in malformed ISOs. Zero-length extents are also skipped in the Lua side so they don't confuse gap aggregation. Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
1 parent e97564b commit 3113760

4 files changed

Lines changed: 135 additions & 49 deletions

File tree

src/core/isoffi.lua

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,12 @@ local function createIsoReaderWrapper(isoReader)
9393
local result = {}
9494
for i = 0, count - 1 do
9595
local name = ffi.string(C.dirEntryName(entries, i))
96-
if name ~= '' and name ~= '\0' and name ~= '\1' then
97-
table.insert(result, {
98-
name = name,
99-
lba = C.dirEntryLBA(entries, i),
100-
size = C.dirEntrySize(entries, i),
101-
isDir = C.dirEntryIsDir(entries, i),
102-
})
103-
end
96+
table.insert(result, {
97+
name = name,
98+
lba = C.dirEntryLBA(entries, i),
99+
size = C.dirEntrySize(entries, i),
100+
isDir = C.dirEntryIsDir(entries, i),
101+
})
104102
end
105103
return result
106104
end,

src/core/luaiso.cc

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include <algorithm>
2323
#include <memory>
24+
#include <unordered_set>
2425

2526
#include "cdrom/cdriso.h"
2627
#include "cdrom/file.h"
@@ -63,11 +64,24 @@ struct DirEntries {
6364
std::vector<PCSX::ISO9660Reader::FullDirEntry> entries;
6465
};
6566

67+
// Drop ISO9660 "." (\0) and ".." (\1) sentinel entries from a listing.
68+
static std::vector<PCSX::ISO9660Reader::FullDirEntry> stripSelfParent(
69+
std::vector<PCSX::ISO9660Reader::FullDirEntry>&& entries) {
70+
std::vector<PCSX::ISO9660Reader::FullDirEntry> out;
71+
out.reserve(entries.size());
72+
for (auto& e : entries) {
73+
const auto& name = e.first.get<PCSX::ISO9660LowLevel::DirEntry_Filename>().value;
74+
if (name.size() == 1 && (name[0] == '\0' || name[0] == '\1')) continue;
75+
out.push_back(std::move(e));
76+
}
77+
return out;
78+
}
79+
6680
DirEntries* readerListDir(PCSX::ISO9660Reader* reader, const char* path) {
6781
if (reader->failed()) return new DirEntries{};
6882
auto root = reader->getRootDirEntry();
6983
if (path == nullptr || path[0] == '\0') {
70-
return new DirEntries{reader->listAllEntriesFrom(root)};
84+
return new DirEntries{stripSelfParent(reader->listAllEntriesFrom(root))};
7185
}
7286
// Walk the path using listAllEntriesFrom. ISO9660 directory entries
7387
// don't carry version suffixes (only files do), so exact match on each
@@ -90,7 +104,7 @@ DirEntries* readerListDir(PCSX::ISO9660Reader* reader, const char* path) {
90104
if ((current.get<PCSX::ISO9660LowLevel::DirEntry_Flags>().value & 2) == 0) {
91105
return new DirEntries{};
92106
}
93-
return new DirEntries{reader->listAllEntriesFrom(current)};
107+
return new DirEntries{stripSelfParent(reader->listAllEntriesFrom(current))};
94108
}
95109

96110
void deleteDirEntries(DirEntries* entries) { delete entries; }
@@ -127,7 +141,8 @@ struct GapList {
127141
static constexpr uint16_t XA_ATTR_FORM2 = 0x1000;
128142

129143
static void collectAllEntries(PCSX::ISO9660Reader* reader, const PCSX::ISO9660LowLevel::DirEntry& dir,
130-
std::vector<std::pair<uint32_t, uint32_t>>& out) {
144+
std::vector<std::pair<uint32_t, uint32_t>>& out,
145+
std::unordered_set<uint32_t>& visitedDirs) {
131146
auto entries = reader->listAllEntriesFrom(dir);
132147
for (auto& [entry, xa] : entries) {
133148
const auto& name = entry.get<PCSX::ISO9660LowLevel::DirEntry_Filename>().value;
@@ -137,9 +152,14 @@ static void collectAllEntries(PCSX::ISO9660Reader* reader, const PCSX::ISO9660Lo
137152
uint16_t attribs = xa.get<PCSX::ISO9660LowLevel::DirEntry_XA_Attribs>();
138153
uint32_t sectorSize = (attribs & XA_ATTR_FORM2) ? 2324 : 2048;
139154
uint32_t sectors = (size + sectorSize - 1) / sectorSize;
140-
out.push_back({lba, sectors});
155+
// Skip zero-length extents: they don't consume sectors, and emitting
156+
// them would confuse the gap aggregation pass.
157+
if (sectors != 0) out.push_back({lba, sectors});
141158
bool isDir = (entry.get<PCSX::ISO9660LowLevel::DirEntry_Flags>().value & 2) != 0;
142-
if (isDir) collectAllEntries(reader, entry, out);
159+
// Guard against malformed ISOs with directory cycles.
160+
if (isDir && visitedDirs.insert(lba).second) {
161+
collectAllEntries(reader, entry, out, visitedDirs);
162+
}
143163
}
144164
}
145165

@@ -167,7 +187,9 @@ GapList* readerFindGaps(PCSX::ISO9660Reader* reader) {
167187
uint32_t rootSize = rootDir.get<PCSX::ISO9660LowLevel::DirEntry_Size>();
168188
allFiles.push_back({rootLBA, (rootSize + 2047) / 2048});
169189

170-
collectAllEntries(reader, reader->getRootDirEntry(), allFiles);
190+
std::unordered_set<uint32_t> visitedDirs;
191+
visitedDirs.insert(rootLBA);
192+
collectAllEntries(reader, reader->getRootDirEntry(), allFiles, visitedDirs);
171193
std::sort(allFiles.begin(), allFiles.end());
172194

173195
auto* result = new GapList{};

0 commit comments

Comments
 (0)