Browse Source

fix(RegionMap): distinguish clean EOF from partial read in load()

Previously load() returned true unconditionally on file-open success,
masking truncated or corrupt /regions2 files. Additionally, the first
field of each entry record (r->id) used the same success-chaining
pattern as subsequent fields, so a clean EOF at a record boundary set
success=false and would have been indistinguishable from real
corruption once the return value was fixed.

The r->id read is now split out: n==0 is a clean EOF (break, success
retains its prior value from the header read), n!=sizeof(r->id) is a
partial read or corruption (break, success=false). load() now returns
success instead of an unconditional true, so its return value reflects
the actual parse outcome.

Companion fix to #2372, which fixed the same return-true hardcoding
in save(). (#1891 originally reported this on load() but was closed
when #2372 landed — that PR only touched save(); this addresses the
load() side.)
pull/2917/head
jirogit 2 weeks ago
parent
commit
8ef369058d
  1. 8
      src/helpers/RegionMap.cpp

8
src/helpers/RegionMap.cpp

@ -93,13 +93,15 @@ bool RegionMap::load(FILESYSTEM* _fs, const char* path) {
while (num_regions < MAX_REGION_ENTRIES) { while (num_regions < MAX_REGION_ENTRIES) {
auto r = &regions[num_regions]; auto r = &regions[num_regions];
success = file.read((uint8_t *) &r->id, sizeof(r->id)) == sizeof(r->id); int n = file.read((uint8_t *) &r->id, sizeof(r->id));
if (n == 0) break; // clean EOF
success = (n == sizeof(r->id));
success = success && file.read((uint8_t *) &r->parent, sizeof(r->parent)) == sizeof(r->parent); success = success && file.read((uint8_t *) &r->parent, sizeof(r->parent)) == sizeof(r->parent);
success = success && file.read((uint8_t *) r->name, sizeof(r->name)) == sizeof(r->name); success = success && file.read((uint8_t *) r->name, sizeof(r->name)) == sizeof(r->name);
success = success && file.read((uint8_t *) &r->flags, sizeof(r->flags)) == sizeof(r->flags); success = success && file.read((uint8_t *) &r->flags, sizeof(r->flags)) == sizeof(r->flags);
success = success && file.read(pad, sizeof(pad)) == sizeof(pad); success = success && file.read(pad, sizeof(pad)) == sizeof(pad);
if (!success) break; // EOF if (!success) break; // partial read or corruption
if (r->id >= next_id) { // make sure next_id is valid if (r->id >= next_id) { // make sure next_id is valid
next_id = r->id + 1; next_id = r->id + 1;
@ -108,7 +110,7 @@ bool RegionMap::load(FILESYSTEM* _fs, const char* path) {
} }
} }
file.close(); file.close();
return true; return success;
} }
} }
return false; // failed return false; // failed

Loading…
Cancel
Save