From 20bc0d80da80337df0ca793055bcbfc572b0939a Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Fri, 29 May 2026 18:02:23 +0000 Subject: [PATCH 1/7] pak readers: bounds-checked decode via a self-contained node_body cursor node_body owns one node's bytes (unique_ptr) plus a bounds-checked cursor and the reader's type name for diagnostics. Each reader's read_node body becomes 'auto p = node_body(fp, node.size, get_type_name()); if (!p) return NULL;' on top of the existing decode_*(p) calls. Free decode_uint*(node_body&) overloads dispatch by argument type, so existing call sites work unchanged while the raw-pointer decode_uint*(char*&) overloads keep serving the node-header reads in pakset_manager.cc. The diagnostic on overrun names the reader and reports offset / total / need / have. Backport onto the upstream tree; bounds-checking only. The loader-level OOM guards (node-size cap, get_name NULL-check) and the bulk-pixel read that rode along with this change upstream are left for a separate pass. --- .../descriptor/reader/bridge_reader.cc | 8 +- .../descriptor/reader/building_reader.cc | 15 +-- .../descriptor/reader/citycar_reader.cc | 8 +- .../descriptor/reader/crossing_reader.cc | 8 +- .../descriptor/reader/factory_reader.cc | 45 ++------ .../descriptor/reader/good_reader.cc | 8 +- .../descriptor/reader/groundobj_reader.cc | 8 +- .../descriptor/reader/image_reader.cc | 26 +++-- .../descriptor/reader/imagelist2d_reader.cc | 8 +- .../descriptor/reader/imagelist_reader.cc | 8 +- src/simutrans/descriptor/reader/obj_reader.h | 107 ++++++++++++++++++ .../descriptor/reader/pedestrian_reader.cc | 8 +- .../descriptor/reader/roadsign_reader.cc | 8 +- .../descriptor/reader/sound_reader.cc | 10 +- .../descriptor/reader/tree_reader.cc | 8 +- .../descriptor/reader/tunnel_reader.cc | 6 +- .../descriptor/reader/vehicle_reader.cc | 8 +- .../descriptor/reader/way_obj_reader.cc | 8 +- src/simutrans/descriptor/reader/way_reader.cc | 8 +- .../descriptor/reader/xref_reader.cc | 20 ++-- 20 files changed, 177 insertions(+), 156 deletions(-) diff --git a/src/simutrans/descriptor/reader/bridge_reader.cc b/src/simutrans/descriptor/reader/bridge_reader.cc index b5135be35e..6c8f012921 100644 --- a/src/simutrans/descriptor/reader/bridge_reader.cc +++ b/src/simutrans/descriptor/reader/bridge_reader.cc @@ -12,7 +12,6 @@ #include "bridge_reader.h" #include "../obj_node_info.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include #include @@ -32,11 +31,8 @@ void bridge_reader_t::register_obj(obj_desc_t *&data) obj_desc_t *bridge_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/building_reader.cc b/src/simutrans/descriptor/reader/building_reader.cc index dc62422e41..11c8f0378b 100644 --- a/src/simutrans/descriptor/reader/building_reader.cc +++ b/src/simutrans/descriptor/reader/building_reader.cc @@ -12,7 +12,6 @@ #include "../obj_node_info.h" #include "building_reader.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" /** @@ -33,11 +32,8 @@ struct old_btyp obj_desc_t * tile_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the highest bit was always cleared. @@ -218,11 +214,8 @@ bool building_reader_t::successfully_loaded() const obj_desc_t *building_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char * p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the highest bit was always cleared. diff --git a/src/simutrans/descriptor/reader/citycar_reader.cc b/src/simutrans/descriptor/reader/citycar_reader.cc index 68821a789e..c40c04a340 100644 --- a/src/simutrans/descriptor/reader/citycar_reader.cc +++ b/src/simutrans/descriptor/reader/citycar_reader.cc @@ -16,7 +16,6 @@ #include "../../simdebug.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void citycar_reader_t::register_obj(obj_desc_t *&data) @@ -39,11 +38,8 @@ bool citycar_reader_t::successfully_loaded() const obj_desc_t * citycar_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/crossing_reader.cc b/src/simutrans/descriptor/reader/crossing_reader.cc index 032e32ab8f..91e3f462a3 100644 --- a/src/simutrans/descriptor/reader/crossing_reader.cc +++ b/src/simutrans/descriptor/reader/crossing_reader.cc @@ -15,7 +15,6 @@ #include "../../simdebug.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void crossing_reader_t::register_obj(obj_desc_t *&data) @@ -33,11 +32,8 @@ void crossing_reader_t::register_obj(obj_desc_t *&data) obj_desc_t * crossing_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/factory_reader.cc b/src/simutrans/descriptor/reader/factory_reader.cc index 4a4d89a608..211141353a 100644 --- a/src/simutrans/descriptor/reader/factory_reader.cc +++ b/src/simutrans/descriptor/reader/factory_reader.cc @@ -12,7 +12,6 @@ #include "../factory_desc.h" #include "../xref_desc.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include "factory_reader.h" @@ -41,11 +40,8 @@ uint16 rescale_probability(const uint16 p) obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; uint16 v = decode_uint16(p); field_class_desc_t *desc = new field_class_desc_t(); @@ -74,11 +70,8 @@ obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &n obj_desc_t *factory_field_group_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; uint16 v = decode_uint16(p); field_group_desc_t *desc = new field_group_desc_t(); @@ -168,11 +161,8 @@ void factory_field_group_reader_t::register_obj(obj_desc_t *&data) obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; sint16 x = decode_sint16(p); sint16 y = decode_sint16(p); @@ -194,16 +184,11 @@ obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node) obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); - + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. - const uint16 v = decode_uint16(p); const int version = v & 0x8000 ? v & 0x7FFF : 0; @@ -232,11 +217,8 @@ obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. @@ -270,11 +252,8 @@ obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node) obj_desc_t *factory_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/good_reader.cc b/src/simutrans/descriptor/reader/good_reader.cc index 1a23f29e3a..5c23c2d953 100644 --- a/src/simutrans/descriptor/reader/good_reader.cc +++ b/src/simutrans/descriptor/reader/good_reader.cc @@ -11,7 +11,6 @@ #include "../obj_node_info.h" #include "../goods_desc.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void goods_reader_t::register_obj(obj_desc_t *&data) @@ -37,11 +36,8 @@ bool goods_reader_t::successfully_loaded() const obj_desc_t * goods_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/groundobj_reader.cc b/src/simutrans/descriptor/reader/groundobj_reader.cc index f535169142..5869a365ff 100644 --- a/src/simutrans/descriptor/reader/groundobj_reader.cc +++ b/src/simutrans/descriptor/reader/groundobj_reader.cc @@ -15,7 +15,6 @@ #include "../obj_node_info.h" #include "groundobj_reader.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include @@ -45,11 +44,8 @@ bool groundobj_reader_t::successfully_loaded() const obj_desc_t *groundobj_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the highest bit was always cleared. diff --git a/src/simutrans/descriptor/reader/image_reader.cc b/src/simutrans/descriptor/reader/image_reader.cc index 6df9ade7eb..267c86fb65 100644 --- a/src/simutrans/descriptor/reader/image_reader.cc +++ b/src/simutrans/descriptor/reader/image_reader.cc @@ -16,7 +16,6 @@ #include #include "../../tpl/inthashtable_tpl.h" -#include "../../tpl/array_tpl.h" // if without graphics backend, do not copy any pixel @@ -28,16 +27,14 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin()+6; + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; - // always zero in old version, since length was always less than 65535 - // because a node could not hold more data + // version byte sits at offset 6; old versions stored 0 there + // since the node size header was uint16 and couldn't reach 65535. + p.seek(6); uint8 version = decode_uint8(p); - p = desc_buf.begin(); + p.seek(0); #if COLOUR_DEPTH != 0 image_t *desc = new image_t(); @@ -47,6 +44,9 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) #endif if(version==0) { + if (node.size < 11) { + dbg->fatal("image_reader_t::read_node", "malformed v0 image node: size %u < 11", node.size); + } desc->x = decode_uint8(p); desc->w = decode_uint8(p); desc->y = decode_uint8(p); @@ -60,7 +60,7 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) //PAKSET_INFO("image_t::read_node()","x,y=%d,%d w,h=%d,%d, len=%i",desc->x,desc->y,desc->w,desc->h, desc->len); uint16* dest = desc->data; - p = desc_buf.begin()+12; + p.seek(12); if (desc->h > 0) { for (uint i = 0; i < desc->len; i++) { @@ -74,6 +74,9 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) } } else if(version<=2) { + if (node.size < 10) { + dbg->fatal("image_reader_t::read_node", "malformed v%u image node: size %u < 10", version, node.size); + } desc->x = decode_sint16(p); desc->y = decode_sint16(p); desc->w = decode_uint8(p); @@ -92,6 +95,9 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) } } else if(version==3) { + if (node.size < 10) { + dbg->fatal("image_reader_t::read_node", "malformed v3 image node: size %u < 10", node.size); + } desc->x = decode_sint16(p); desc->y = decode_sint16(p); desc->w = decode_sint16(p); diff --git a/src/simutrans/descriptor/reader/imagelist2d_reader.cc b/src/simutrans/descriptor/reader/imagelist2d_reader.cc index 8487bd6726..0a594594db 100644 --- a/src/simutrans/descriptor/reader/imagelist2d_reader.cc +++ b/src/simutrans/descriptor/reader/imagelist2d_reader.cc @@ -10,16 +10,12 @@ #include "imagelist2d_reader.h" #include "../obj_node_info.h" -#include "../../tpl/array_tpl.h" obj_desc_t * imagelist2d_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; image_array_t *desc = new image_array_t(); desc->count = decode_uint16(p); diff --git a/src/simutrans/descriptor/reader/imagelist_reader.cc b/src/simutrans/descriptor/reader/imagelist_reader.cc index a5ca577627..1338e1426b 100644 --- a/src/simutrans/descriptor/reader/imagelist_reader.cc +++ b/src/simutrans/descriptor/reader/imagelist_reader.cc @@ -10,16 +10,12 @@ #include "imagelist_reader.h" #include "../obj_node_info.h" -#include "../../tpl/array_tpl.h" obj_desc_t * imagelist_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; image_list_t *desc = new image_list_t(); desc->count = decode_uint16(p); diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h index 8893b42a1e..db093a7c31 100644 --- a/src/simutrans/descriptor/reader/obj_reader.h +++ b/src/simutrans/descriptor/reader/obj_reader.h @@ -8,6 +8,7 @@ #include +#include #include "../obj_node_info.h" #include "../objversion.h" @@ -96,6 +97,112 @@ inline uint64 decode_uint64(char *&data) static classname the_instance +/// Owns the bytes of one pak node body plus a bounds-checked cursor +/// over them. Constructed in each reader's read_node() with +/// `auto p = node_body(fp, node.size, get_type_name());`; +/// decode_*(node_body&) free overloads dispatch to its bounds-checked +/// readers. +class node_body +{ +public: + /// Allocate @p size bytes, fread them from @p fp. Short fread + /// leaves the cursor in failure state (bool-false). + /// @p size == 0 yields a valid but empty cursor. + node_body(FILE* fp, size_t size, const char* type_name) + : buf_(new char[size > 0 ? size : 1]), + pos_(NULL), + end_(NULL), + type_name_(type_name) + { + if (size > 0 && fread(buf_.get(), size, 1, fp) != 1) { + buf_.reset(); + return; + } + pos_ = buf_.get(); + end_ = buf_.get() + size; + } + + /// false on short fread. + explicit operator bool() const { return pos_ != NULL; } + + /// Bounds-checked absolute seek from the buffer start. + void seek(size_t off) + { + if (buf_.get() + off > end_) { + dbg->fatal(type_name_, + "seek to offset %zu past buffer end %zu", + off, (size_t)(end_ - buf_.get())); + } + pos_ = buf_.get() + off; + } + + /// Bounds-checked skip-forward; pairs with the legacy + /// `p += N` idiom at reader call sites. + node_body& operator+=(size_t n) { require(n); pos_ += n; return *this; } + + /// Single-byte skip — `p++` for callers that used to advance a + /// raw char* by one byte (typically to step over a version tag). + /// Returns *this rather than a pre-increment copy because the + /// owned unique_ptr buffer can't be cheaply duplicated; no current + /// caller reads the return value. + node_body& operator++(int) { require(1); ++pos_; return *this; } + + /// Return the current cursor and advance by @p n bytes + /// (bounds-checked). Used for memcpy-style tail reads. + char* read_bytes(size_t n) { require(n); char* p = pos_; pos_ += n; return p; } + + uint8 read_uint8() { require(1); return (uint8)*pos_++; } + uint16 read_uint16() { require(2); uint16 v = (uint16)(uint8)pos_[0] | (uint16)(uint8)pos_[1] << 8; pos_ += 2; return v; } + uint32 read_uint32() + { + require(4); + uint32 v = + (uint32)(uint8)pos_[0] | + (uint32)(uint8)pos_[1] << 8 | + (uint32)(uint8)pos_[2] << 16 | + (uint32)(uint8)pos_[3] << 24; + pos_ += 4; + return v; + } + uint64 read_uint64() + { + require(8); + uint64 v = 0; + for (int i = 0; i < 8; ++i) { + v |= (uint64)(uint8)pos_[i] << (i * 8); + } + pos_ += 8; + return v; + } + +private: + void require(size_t n) const + { + if (pos_ + n > end_) { + dbg->fatal(type_name_, + "short read at offset %zu of %zu: need %zu, have %zu", + (size_t)(pos_ - buf_.get()), (size_t)(end_ - buf_.get()), + n, (size_t)(end_ - pos_)); + } + } + + std::unique_ptr buf_; + char* pos_; + char* end_; + const char* type_name_; +}; + +/// Free decode_uint*() overloads that dispatch to the cursor's +/// bounds-checked readers. Paired with the free char*& overloads +/// above: `decode_uint16(p)` resolves to either based on whether +/// `p` is a node_body (reader bodies) or char* (pakset_manager +/// node-header reads). +inline uint8 decode_uint8(node_body& b) { return b.read_uint8(); } +inline uint16 decode_uint16(node_body& b) { return b.read_uint16(); } +inline uint32 decode_uint32(node_body& b) { return b.read_uint32(); } +inline uint64 decode_uint64(node_body& b) { return b.read_uint64(); } + + class obj_reader_t { public: diff --git a/src/simutrans/descriptor/reader/pedestrian_reader.cc b/src/simutrans/descriptor/reader/pedestrian_reader.cc index 7eccbde7bc..878d005a8d 100644 --- a/src/simutrans/descriptor/reader/pedestrian_reader.cc +++ b/src/simutrans/descriptor/reader/pedestrian_reader.cc @@ -12,7 +12,6 @@ #include "pedestrian_reader.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void pedestrian_reader_t::register_obj(obj_desc_t *&data) @@ -39,11 +38,8 @@ bool pedestrian_reader_t::successfully_loaded() const */ obj_desc_t * pedestrian_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/roadsign_reader.cc b/src/simutrans/descriptor/reader/roadsign_reader.cc index 48f48731de..b53af7e0c0 100644 --- a/src/simutrans/descriptor/reader/roadsign_reader.cc +++ b/src/simutrans/descriptor/reader/roadsign_reader.cc @@ -15,7 +15,6 @@ #include "../../simdebug.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include @@ -40,11 +39,8 @@ bool roadsign_reader_t::successfully_loaded() const obj_desc_t *roadsign_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; const uint16 v = decode_uint16(p); const int version = v & 0x8000 ? v & 0x7FFF : 0; diff --git a/src/simutrans/descriptor/reader/sound_reader.cc b/src/simutrans/descriptor/reader/sound_reader.cc index 304ae7d7f6..608e045fc6 100644 --- a/src/simutrans/descriptor/reader/sound_reader.cc +++ b/src/simutrans/descriptor/reader/sound_reader.cc @@ -11,7 +11,6 @@ #include "../obj_node_info.h" #include "../../simdebug.h" -#include "../../tpl/array_tpl.h" void sound_reader_t::register_obj(obj_desc_t *&data) @@ -25,11 +24,8 @@ void sound_reader_t::register_obj(obj_desc_t *&data) obj_desc_t * sound_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; const uint16 v = decode_uint16(p); const int version = v & 0x8000 ? v & 0x7FFF : 0; @@ -45,7 +41,7 @@ obj_desc_t * sound_reader_t::read_node(FILE *fp, obj_node_info_t &node) desc->nr = decode_uint16(p); uint16 len = decode_uint16(p); if( len>0 ) { - desc->nr = desc->get_sound_id(p); + desc->nr = desc->get_sound_id(p.read_bytes(len)); } } else { diff --git a/src/simutrans/descriptor/reader/tree_reader.cc b/src/simutrans/descriptor/reader/tree_reader.cc index 5f7c065af5..4dff03a4a9 100644 --- a/src/simutrans/descriptor/reader/tree_reader.cc +++ b/src/simutrans/descriptor/reader/tree_reader.cc @@ -13,7 +13,6 @@ #include "../obj_node_info.h" #include "tree_reader.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void tree_reader_t::register_obj(obj_desc_t *&data) @@ -35,11 +34,8 @@ bool tree_reader_t::successfully_loaded() const obj_desc_t * tree_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the highest bit was always cleared. diff --git a/src/simutrans/descriptor/reader/tunnel_reader.cc b/src/simutrans/descriptor/reader/tunnel_reader.cc index 0573776cea..45b8937d2f 100644 --- a/src/simutrans/descriptor/reader/tunnel_reader.cc +++ b/src/simutrans/descriptor/reader/tunnel_reader.cc @@ -17,7 +17,6 @@ #include "../../builder/tunnelbauer.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include @@ -67,12 +66,11 @@ obj_desc_t * tunnel_reader_t::read_node(FILE *fp, obj_node_info_t &node) return desc; } - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { + auto p = node_body(fp, node.size, get_type_name()); + if (!p) { delete desc; return NULL; } - char *p = desc_buf.begin(); const uint16 v = decode_uint16(p); const uint16 version = v & 0x8000 ? v & 0x7FFF : 0; diff --git a/src/simutrans/descriptor/reader/vehicle_reader.cc b/src/simutrans/descriptor/reader/vehicle_reader.cc index cc90b7d682..5492403ba3 100644 --- a/src/simutrans/descriptor/reader/vehicle_reader.cc +++ b/src/simutrans/descriptor/reader/vehicle_reader.cc @@ -14,7 +14,6 @@ #include "vehicle_reader.h" #include "../obj_node_info.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void vehicle_reader_t::register_obj(obj_desc_t *&data) @@ -37,11 +36,8 @@ bool vehicle_reader_t::successfully_loaded() const obj_desc_t *vehicle_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/way_obj_reader.cc b/src/simutrans/descriptor/reader/way_obj_reader.cc index e897c15543..a1e59ccace 100644 --- a/src/simutrans/descriptor/reader/way_obj_reader.cc +++ b/src/simutrans/descriptor/reader/way_obj_reader.cc @@ -13,7 +13,6 @@ #include "way_obj_reader.h" #include "../obj_node_info.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" #include @@ -37,11 +36,8 @@ bool way_obj_reader_t::successfully_loaded() const obj_desc_t * way_obj_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } - char *p = desc_buf.begin(); + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; // old versions of PAK files have no version stamp. // But we know, the higher most bit was always cleared. diff --git a/src/simutrans/descriptor/reader/way_reader.cc b/src/simutrans/descriptor/reader/way_reader.cc index 7b1290f9a6..ce8123624c 100644 --- a/src/simutrans/descriptor/reader/way_reader.cc +++ b/src/simutrans/descriptor/reader/way_reader.cc @@ -14,7 +14,6 @@ #include "way_reader.h" #include "../obj_node_info.h" #include "../../network/pakset_info.h" -#include "../../tpl/array_tpl.h" void way_reader_t::register_obj(obj_desc_t *&data) @@ -38,12 +37,9 @@ bool way_reader_t::successfully_loaded() const obj_desc_t * way_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - array_tpl desc_buf(node.size); - if (fread(desc_buf.begin(), node.size, 1, fp) != 1) { - return NULL; - } + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; - char *p = desc_buf.begin(); way_desc_t *desc = new way_desc_t; const uint16 version = node.size==0 ? 0 : decode_uint16(p)&0x7FFFu; diff --git a/src/simutrans/descriptor/reader/xref_reader.cc b/src/simutrans/descriptor/reader/xref_reader.cc index a8c97b7a2a..672a4c3193 100644 --- a/src/simutrans/descriptor/reader/xref_reader.cc +++ b/src/simutrans/descriptor/reader/xref_reader.cc @@ -4,6 +4,7 @@ */ #include +#include #include "../../simdebug.h" #include "../xref_desc.h" #include "xref_reader.h" @@ -13,25 +14,18 @@ obj_desc_t *xref_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - char buf[4 + 1]; - if (fread(buf, 1, 5, fp) != 5) { + if (node.size < 5) { + dbg->error("xref_reader_t::read_node", "node.size %u < 5", node.size); return NULL; } + auto p = node_body(fp, node.size, get_type_name()); + if (!p) return NULL; - const uint32 name_len = node.size - 4 - 1; - char *p = buf; + const uint32 name_len = node.size - 5; xref_desc_t* desc = new(name_len) xref_desc_t(); - desc->type = static_cast(decode_uint32(p)); desc->fatal = (decode_uint8(p) != 0); - - if (fread(desc->name, 1, name_len, fp) != name_len) { - delete desc; - return NULL; - } - -// PAKSET_INFO("xref_reader_t::read_node()", "%s",desc->get_text() ); - + memcpy(desc->name, p.read_bytes(name_len), name_len); return desc; } From 0471e0119cd063afc437f4e8b7a3bd30b393ebcf Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Fri, 29 May 2026 11:20:54 +0000 Subject: [PATCH 2/7] hex-port: drop auto for node_body locals in pakset readers --- src/simutrans/descriptor/reader/bridge_reader.cc | 2 +- src/simutrans/descriptor/reader/building_reader.cc | 4 ++-- src/simutrans/descriptor/reader/citycar_reader.cc | 2 +- src/simutrans/descriptor/reader/crossing_reader.cc | 2 +- src/simutrans/descriptor/reader/factory_reader.cc | 12 ++++++------ src/simutrans/descriptor/reader/good_reader.cc | 2 +- src/simutrans/descriptor/reader/groundobj_reader.cc | 2 +- src/simutrans/descriptor/reader/image_reader.cc | 2 +- .../descriptor/reader/imagelist2d_reader.cc | 2 +- src/simutrans/descriptor/reader/imagelist_reader.cc | 2 +- src/simutrans/descriptor/reader/obj_reader.h | 2 +- src/simutrans/descriptor/reader/pedestrian_reader.cc | 2 +- src/simutrans/descriptor/reader/roadsign_reader.cc | 2 +- src/simutrans/descriptor/reader/sound_reader.cc | 2 +- src/simutrans/descriptor/reader/tree_reader.cc | 2 +- src/simutrans/descriptor/reader/tunnel_reader.cc | 2 +- src/simutrans/descriptor/reader/vehicle_reader.cc | 2 +- src/simutrans/descriptor/reader/way_obj_reader.cc | 2 +- src/simutrans/descriptor/reader/way_reader.cc | 2 +- src/simutrans/descriptor/reader/xref_reader.cc | 2 +- 20 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/simutrans/descriptor/reader/bridge_reader.cc b/src/simutrans/descriptor/reader/bridge_reader.cc index 6c8f012921..971eb49eeb 100644 --- a/src/simutrans/descriptor/reader/bridge_reader.cc +++ b/src/simutrans/descriptor/reader/bridge_reader.cc @@ -31,7 +31,7 @@ void bridge_reader_t::register_obj(obj_desc_t *&data) obj_desc_t *bridge_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/building_reader.cc b/src/simutrans/descriptor/reader/building_reader.cc index 11c8f0378b..d13a663f46 100644 --- a/src/simutrans/descriptor/reader/building_reader.cc +++ b/src/simutrans/descriptor/reader/building_reader.cc @@ -32,7 +32,7 @@ struct old_btyp obj_desc_t * tile_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. @@ -214,7 +214,7 @@ bool building_reader_t::successfully_loaded() const obj_desc_t *building_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/citycar_reader.cc b/src/simutrans/descriptor/reader/citycar_reader.cc index c40c04a340..d8eeb4ea81 100644 --- a/src/simutrans/descriptor/reader/citycar_reader.cc +++ b/src/simutrans/descriptor/reader/citycar_reader.cc @@ -38,7 +38,7 @@ bool citycar_reader_t::successfully_loaded() const obj_desc_t * citycar_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/crossing_reader.cc b/src/simutrans/descriptor/reader/crossing_reader.cc index 91e3f462a3..ea1f064e2b 100644 --- a/src/simutrans/descriptor/reader/crossing_reader.cc +++ b/src/simutrans/descriptor/reader/crossing_reader.cc @@ -32,7 +32,7 @@ void crossing_reader_t::register_obj(obj_desc_t *&data) obj_desc_t * crossing_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/factory_reader.cc b/src/simutrans/descriptor/reader/factory_reader.cc index 211141353a..cbe7dcf86c 100644 --- a/src/simutrans/descriptor/reader/factory_reader.cc +++ b/src/simutrans/descriptor/reader/factory_reader.cc @@ -40,7 +40,7 @@ uint16 rescale_probability(const uint16 p) obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; uint16 v = decode_uint16(p); @@ -70,7 +70,7 @@ obj_desc_t *factory_field_class_reader_t::read_node(FILE *fp, obj_node_info_t &n obj_desc_t *factory_field_group_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; uint16 v = decode_uint16(p); @@ -161,7 +161,7 @@ void factory_field_group_reader_t::register_obj(obj_desc_t *&data) obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; sint16 x = decode_sint16(p); @@ -184,7 +184,7 @@ obj_desc_t *factory_smoke_reader_t::read_node(FILE *fp, obj_node_info_t &node) obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. @@ -217,7 +217,7 @@ obj_desc_t *factory_supplier_reader_t::read_node(FILE *fp, obj_node_info_t &node obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. @@ -252,7 +252,7 @@ obj_desc_t *factory_product_reader_t::read_node(FILE *fp, obj_node_info_t &node) obj_desc_t *factory_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/good_reader.cc b/src/simutrans/descriptor/reader/good_reader.cc index 5c23c2d953..082650b6a9 100644 --- a/src/simutrans/descriptor/reader/good_reader.cc +++ b/src/simutrans/descriptor/reader/good_reader.cc @@ -36,7 +36,7 @@ bool goods_reader_t::successfully_loaded() const obj_desc_t * goods_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/groundobj_reader.cc b/src/simutrans/descriptor/reader/groundobj_reader.cc index 5869a365ff..50b3ab06c7 100644 --- a/src/simutrans/descriptor/reader/groundobj_reader.cc +++ b/src/simutrans/descriptor/reader/groundobj_reader.cc @@ -44,7 +44,7 @@ bool groundobj_reader_t::successfully_loaded() const obj_desc_t *groundobj_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/image_reader.cc b/src/simutrans/descriptor/reader/image_reader.cc index 267c86fb65..3c8e9b9f99 100644 --- a/src/simutrans/descriptor/reader/image_reader.cc +++ b/src/simutrans/descriptor/reader/image_reader.cc @@ -27,7 +27,7 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // version byte sits at offset 6; old versions stored 0 there diff --git a/src/simutrans/descriptor/reader/imagelist2d_reader.cc b/src/simutrans/descriptor/reader/imagelist2d_reader.cc index 0a594594db..d4be3204d9 100644 --- a/src/simutrans/descriptor/reader/imagelist2d_reader.cc +++ b/src/simutrans/descriptor/reader/imagelist2d_reader.cc @@ -14,7 +14,7 @@ obj_desc_t * imagelist2d_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; image_array_t *desc = new image_array_t(); diff --git a/src/simutrans/descriptor/reader/imagelist_reader.cc b/src/simutrans/descriptor/reader/imagelist_reader.cc index 1338e1426b..6a9bfa8045 100644 --- a/src/simutrans/descriptor/reader/imagelist_reader.cc +++ b/src/simutrans/descriptor/reader/imagelist_reader.cc @@ -14,7 +14,7 @@ obj_desc_t * imagelist_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; image_list_t *desc = new image_list_t(); diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h index db093a7c31..afc52eff37 100644 --- a/src/simutrans/descriptor/reader/obj_reader.h +++ b/src/simutrans/descriptor/reader/obj_reader.h @@ -99,7 +99,7 @@ inline uint64 decode_uint64(char *&data) /// Owns the bytes of one pak node body plus a bounds-checked cursor /// over them. Constructed in each reader's read_node() with -/// `auto p = node_body(fp, node.size, get_type_name());`; +/// `node_body p(fp, node.size, get_type_name());`; /// decode_*(node_body&) free overloads dispatch to its bounds-checked /// readers. class node_body diff --git a/src/simutrans/descriptor/reader/pedestrian_reader.cc b/src/simutrans/descriptor/reader/pedestrian_reader.cc index 878d005a8d..5993c330f6 100644 --- a/src/simutrans/descriptor/reader/pedestrian_reader.cc +++ b/src/simutrans/descriptor/reader/pedestrian_reader.cc @@ -38,7 +38,7 @@ bool pedestrian_reader_t::successfully_loaded() const */ obj_desc_t * pedestrian_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/roadsign_reader.cc b/src/simutrans/descriptor/reader/roadsign_reader.cc index b53af7e0c0..f6d0afe1c1 100644 --- a/src/simutrans/descriptor/reader/roadsign_reader.cc +++ b/src/simutrans/descriptor/reader/roadsign_reader.cc @@ -39,7 +39,7 @@ bool roadsign_reader_t::successfully_loaded() const obj_desc_t *roadsign_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; const uint16 v = decode_uint16(p); diff --git a/src/simutrans/descriptor/reader/sound_reader.cc b/src/simutrans/descriptor/reader/sound_reader.cc index 608e045fc6..8a05c46efb 100644 --- a/src/simutrans/descriptor/reader/sound_reader.cc +++ b/src/simutrans/descriptor/reader/sound_reader.cc @@ -24,7 +24,7 @@ void sound_reader_t::register_obj(obj_desc_t *&data) obj_desc_t * sound_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; const uint16 v = decode_uint16(p); diff --git a/src/simutrans/descriptor/reader/tree_reader.cc b/src/simutrans/descriptor/reader/tree_reader.cc index 4dff03a4a9..7889f5067c 100644 --- a/src/simutrans/descriptor/reader/tree_reader.cc +++ b/src/simutrans/descriptor/reader/tree_reader.cc @@ -34,7 +34,7 @@ bool tree_reader_t::successfully_loaded() const obj_desc_t * tree_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/tunnel_reader.cc b/src/simutrans/descriptor/reader/tunnel_reader.cc index 45b8937d2f..797558615c 100644 --- a/src/simutrans/descriptor/reader/tunnel_reader.cc +++ b/src/simutrans/descriptor/reader/tunnel_reader.cc @@ -66,7 +66,7 @@ obj_desc_t * tunnel_reader_t::read_node(FILE *fp, obj_node_info_t &node) return desc; } - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) { delete desc; return NULL; diff --git a/src/simutrans/descriptor/reader/vehicle_reader.cc b/src/simutrans/descriptor/reader/vehicle_reader.cc index 5492403ba3..dc4b776e09 100644 --- a/src/simutrans/descriptor/reader/vehicle_reader.cc +++ b/src/simutrans/descriptor/reader/vehicle_reader.cc @@ -36,7 +36,7 @@ bool vehicle_reader_t::successfully_loaded() const obj_desc_t *vehicle_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/way_obj_reader.cc b/src/simutrans/descriptor/reader/way_obj_reader.cc index a1e59ccace..251f8a543b 100644 --- a/src/simutrans/descriptor/reader/way_obj_reader.cc +++ b/src/simutrans/descriptor/reader/way_obj_reader.cc @@ -36,7 +36,7 @@ bool way_obj_reader_t::successfully_loaded() const obj_desc_t * way_obj_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; // old versions of PAK files have no version stamp. diff --git a/src/simutrans/descriptor/reader/way_reader.cc b/src/simutrans/descriptor/reader/way_reader.cc index ce8123624c..63c0e27c5b 100644 --- a/src/simutrans/descriptor/reader/way_reader.cc +++ b/src/simutrans/descriptor/reader/way_reader.cc @@ -37,7 +37,7 @@ bool way_reader_t::successfully_loaded() const obj_desc_t * way_reader_t::read_node(FILE *fp, obj_node_info_t &node) { - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; way_desc_t *desc = new way_desc_t; diff --git a/src/simutrans/descriptor/reader/xref_reader.cc b/src/simutrans/descriptor/reader/xref_reader.cc index 672a4c3193..0534ffdc38 100644 --- a/src/simutrans/descriptor/reader/xref_reader.cc +++ b/src/simutrans/descriptor/reader/xref_reader.cc @@ -18,7 +18,7 @@ obj_desc_t *xref_reader_t::read_node(FILE *fp, obj_node_info_t &node) dbg->error("xref_reader_t::read_node", "node.size %u < 5", node.size); return NULL; } - auto p = node_body(fp, node.size, get_type_name()); + node_body p(fp, node.size, get_type_name()); if (!p) return NULL; const uint32 name_len = node.size - 5; From cd0f67523d6066864974afbd0369d27cf65bf016 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 18:51:32 +0000 Subject: [PATCH 3/7] pak readers: back node_body buffer on array_tpl Replaces the unique_ptr the cursor owned with the array_tpl the readers used before this change. Same heap allocation and RAII free, so the upstream-review objection about a leaking/heap-owning buffer no longer applies; the cursor stays a transparent bounds-checking shim over the same bytes. --- src/simutrans/descriptor/reader/obj_reader.h | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h index afc52eff37..5e9b051e3f 100644 --- a/src/simutrans/descriptor/reader/obj_reader.h +++ b/src/simutrans/descriptor/reader/obj_reader.h @@ -8,12 +8,12 @@ #include -#include #include "../obj_node_info.h" #include "../objversion.h" #include "../../simdebug.h" #include "../../simtypes.h" +#include "../../tpl/array_tpl.h" #include "../../dataobj/pakset_manager.h" @@ -109,17 +109,17 @@ class node_body /// leaves the cursor in failure state (bool-false). /// @p size == 0 yields a valid but empty cursor. node_body(FILE* fp, size_t size, const char* type_name) - : buf_(new char[size > 0 ? size : 1]), + : buf_(size), pos_(NULL), end_(NULL), type_name_(type_name) { - if (size > 0 && fread(buf_.get(), size, 1, fp) != 1) { - buf_.reset(); + if (size > 0 && fread(buf_.begin(), size, 1, fp) != 1) { + buf_.clear(); return; } - pos_ = buf_.get(); - end_ = buf_.get() + size; + pos_ = buf_.begin(); + end_ = buf_.begin() + size; } /// false on short fread. @@ -128,12 +128,12 @@ class node_body /// Bounds-checked absolute seek from the buffer start. void seek(size_t off) { - if (buf_.get() + off > end_) { + if (buf_.begin() + off > end_) { dbg->fatal(type_name_, "seek to offset %zu past buffer end %zu", - off, (size_t)(end_ - buf_.get())); + off, (size_t)(end_ - buf_.begin())); } - pos_ = buf_.get() + off; + pos_ = buf_.begin() + off; } /// Bounds-checked skip-forward; pairs with the legacy @@ -143,7 +143,7 @@ class node_body /// Single-byte skip — `p++` for callers that used to advance a /// raw char* by one byte (typically to step over a version tag). /// Returns *this rather than a pre-increment copy because the - /// owned unique_ptr buffer can't be cheaply duplicated; no current + /// owned array_tpl buffer can't be cheaply duplicated; no current /// caller reads the return value. node_body& operator++(int) { require(1); ++pos_; return *this; } @@ -181,12 +181,12 @@ class node_body if (pos_ + n > end_) { dbg->fatal(type_name_, "short read at offset %zu of %zu: need %zu, have %zu", - (size_t)(pos_ - buf_.get()), (size_t)(end_ - buf_.get()), + (size_t)(pos_ - buf_.begin()), (size_t)(end_ - buf_.begin()), n, (size_t)(end_ - pos_)); } } - std::unique_ptr buf_; + array_tpl buf_; char* pos_; char* end_; const char* type_name_; From 093e9508ec94775fac1be886b9aa6e8fd98408a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 19:03:36 +0000 Subject: [PATCH 4/7] pak readers: trim node_body comments, drop redundant image size guards The node_body doc comments were carrying more prose than the code; cut them to one line each. The image_reader node.size < N guards duplicated the cursor's own bounds check (every header read past node.size already fatals), so they only added a second diagnostic path -- removed. The xref_reader node.size < 5 guard stays: it prevents a name_len underflow that would hit the allocation before any cursor read. --- .../descriptor/reader/image_reader.cc | 9 ------ src/simutrans/descriptor/reader/obj_reader.h | 30 +++++-------------- 2 files changed, 8 insertions(+), 31 deletions(-) diff --git a/src/simutrans/descriptor/reader/image_reader.cc b/src/simutrans/descriptor/reader/image_reader.cc index 3c8e9b9f99..1b38c69e98 100644 --- a/src/simutrans/descriptor/reader/image_reader.cc +++ b/src/simutrans/descriptor/reader/image_reader.cc @@ -44,9 +44,6 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) #endif if(version==0) { - if (node.size < 11) { - dbg->fatal("image_reader_t::read_node", "malformed v0 image node: size %u < 11", node.size); - } desc->x = decode_uint8(p); desc->w = decode_uint8(p); desc->y = decode_uint8(p); @@ -74,9 +71,6 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) } } else if(version<=2) { - if (node.size < 10) { - dbg->fatal("image_reader_t::read_node", "malformed v%u image node: size %u < 10", version, node.size); - } desc->x = decode_sint16(p); desc->y = decode_sint16(p); desc->w = decode_uint8(p); @@ -95,9 +89,6 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) } } else if(version==3) { - if (node.size < 10) { - dbg->fatal("image_reader_t::read_node", "malformed v3 image node: size %u < 10", node.size); - } desc->x = decode_sint16(p); desc->y = decode_sint16(p); desc->w = decode_sint16(p); diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h index 5e9b051e3f..4e8ffbf860 100644 --- a/src/simutrans/descriptor/reader/obj_reader.h +++ b/src/simutrans/descriptor/reader/obj_reader.h @@ -97,17 +97,12 @@ inline uint64 decode_uint64(char *&data) static classname the_instance -/// Owns the bytes of one pak node body plus a bounds-checked cursor -/// over them. Constructed in each reader's read_node() with -/// `node_body p(fp, node.size, get_type_name());`; -/// decode_*(node_body&) free overloads dispatch to its bounds-checked -/// readers. +/// Owns one pak node's bytes and a bounds-checked cursor over them. +/// decode_*(node_body&) overloads read through it. class node_body { public: - /// Allocate @p size bytes, fread them from @p fp. Short fread - /// leaves the cursor in failure state (bool-false). - /// @p size == 0 yields a valid but empty cursor. + /// fread @p size bytes from @p fp; short read leaves a bool-false cursor. node_body(FILE* fp, size_t size, const char* type_name) : buf_(size), pos_(NULL), @@ -136,19 +131,13 @@ class node_body pos_ = buf_.begin() + off; } - /// Bounds-checked skip-forward; pairs with the legacy - /// `p += N` idiom at reader call sites. + /// Bounds-checked `p += n`. node_body& operator+=(size_t n) { require(n); pos_ += n; return *this; } - /// Single-byte skip — `p++` for callers that used to advance a - /// raw char* by one byte (typically to step over a version tag). - /// Returns *this rather than a pre-increment copy because the - /// owned array_tpl buffer can't be cheaply duplicated; no current - /// caller reads the return value. + /// Bounds-checked single-byte skip (`p++`); return value unused. node_body& operator++(int) { require(1); ++pos_; return *this; } - /// Return the current cursor and advance by @p n bytes - /// (bounds-checked). Used for memcpy-style tail reads. + /// Bounds-checked: returns the cursor, then advances @p n bytes (tail reads). char* read_bytes(size_t n) { require(n); char* p = pos_; pos_ += n; return p; } uint8 read_uint8() { require(1); return (uint8)*pos_++; } @@ -192,11 +181,8 @@ class node_body const char* type_name_; }; -/// Free decode_uint*() overloads that dispatch to the cursor's -/// bounds-checked readers. Paired with the free char*& overloads -/// above: `decode_uint16(p)` resolves to either based on whether -/// `p` is a node_body (reader bodies) or char* (pakset_manager -/// node-header reads). +/// decode_*(node_body&) overloads, chosen over the char*& ones by +/// argument type so reader call sites stay `decode_uint16(p)`. inline uint8 decode_uint8(node_body& b) { return b.read_uint8(); } inline uint16 decode_uint16(node_body& b) { return b.read_uint16(); } inline uint32 decode_uint32(node_body& b) { return b.read_uint32(); } From b0a01e402c89c85982e25bb726cf6cb76093f3a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 19:06:14 +0000 Subject: [PATCH 5/7] pak readers: drop node_body end_ cache, derive from buffer end_ was just buf_.begin() + size, which array_tpl already exposes as end() / get_count(). Removing the member drops a redundant copy of the buffer extent; bounds checks now read it from buf_ directly. --- src/simutrans/descriptor/reader/obj_reader.h | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/simutrans/descriptor/reader/obj_reader.h b/src/simutrans/descriptor/reader/obj_reader.h index 4e8ffbf860..b999a4c2c7 100644 --- a/src/simutrans/descriptor/reader/obj_reader.h +++ b/src/simutrans/descriptor/reader/obj_reader.h @@ -106,7 +106,6 @@ class node_body node_body(FILE* fp, size_t size, const char* type_name) : buf_(size), pos_(NULL), - end_(NULL), type_name_(type_name) { if (size > 0 && fread(buf_.begin(), size, 1, fp) != 1) { @@ -114,7 +113,6 @@ class node_body return; } pos_ = buf_.begin(); - end_ = buf_.begin() + size; } /// false on short fread. @@ -123,10 +121,10 @@ class node_body /// Bounds-checked absolute seek from the buffer start. void seek(size_t off) { - if (buf_.begin() + off > end_) { + if (buf_.begin() + off > buf_.end()) { dbg->fatal(type_name_, "seek to offset %zu past buffer end %zu", - off, (size_t)(end_ - buf_.begin())); + off, (size_t)buf_.get_count()); } pos_ = buf_.begin() + off; } @@ -167,17 +165,16 @@ class node_body private: void require(size_t n) const { - if (pos_ + n > end_) { + if (pos_ + n > buf_.end()) { dbg->fatal(type_name_, "short read at offset %zu of %zu: need %zu, have %zu", - (size_t)(pos_ - buf_.begin()), (size_t)(end_ - buf_.begin()), - n, (size_t)(end_ - pos_)); + (size_t)(pos_ - buf_.begin()), (size_t)buf_.get_count(), + n, (size_t)(buf_.end() - pos_)); } } array_tpl buf_; char* pos_; - char* end_; const char* type_name_; }; From 04df56b35f5c4ce44d8f5d947e0405f889e49b3f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 19:08:06 +0000 Subject: [PATCH 6/7] pak readers: keep xref name_len as node.size - 4 - 1 Reverts a pointless reflow to node.size - 5; same value, and the 4 - 1 form documents the 4-byte type plus 1-byte fatal-flag layout. --- src/simutrans/descriptor/reader/xref_reader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/simutrans/descriptor/reader/xref_reader.cc b/src/simutrans/descriptor/reader/xref_reader.cc index 0534ffdc38..ada06609f5 100644 --- a/src/simutrans/descriptor/reader/xref_reader.cc +++ b/src/simutrans/descriptor/reader/xref_reader.cc @@ -21,7 +21,7 @@ obj_desc_t *xref_reader_t::read_node(FILE *fp, obj_node_info_t &node) node_body p(fp, node.size, get_type_name()); if (!p) return NULL; - const uint32 name_len = node.size - 5; + const uint32 name_len = node.size - 4 - 1; xref_desc_t* desc = new(name_len) xref_desc_t(); desc->type = static_cast(decode_uint32(p)); desc->fatal = (decode_uint8(p) != 0); From c9d89059eeead230c21bced0430e42d32849bd78 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 19:12:09 +0000 Subject: [PATCH 7/7] pak readers: keep upstream comments in image/xref readers Restores the image_reader version-byte comment verbatim and the commented-out PAKSET_INFO line in xref_reader; neither was touched by the refactor, so rewriting/dropping them was needless churn. --- src/simutrans/descriptor/reader/image_reader.cc | 4 ++-- src/simutrans/descriptor/reader/xref_reader.cc | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/simutrans/descriptor/reader/image_reader.cc b/src/simutrans/descriptor/reader/image_reader.cc index 1b38c69e98..884a7483f8 100644 --- a/src/simutrans/descriptor/reader/image_reader.cc +++ b/src/simutrans/descriptor/reader/image_reader.cc @@ -30,9 +30,9 @@ obj_desc_t *image_reader_t::read_node(FILE *fp, obj_node_info_t &node) node_body p(fp, node.size, get_type_name()); if (!p) return NULL; - // version byte sits at offset 6; old versions stored 0 there - // since the node size header was uint16 and couldn't reach 65535. p.seek(6); + // always zero in old version, since length was always less than 65535 + // because a node could not hold more data uint8 version = decode_uint8(p); p.seek(0); diff --git a/src/simutrans/descriptor/reader/xref_reader.cc b/src/simutrans/descriptor/reader/xref_reader.cc index ada06609f5..aaa40bed4a 100644 --- a/src/simutrans/descriptor/reader/xref_reader.cc +++ b/src/simutrans/descriptor/reader/xref_reader.cc @@ -26,6 +26,9 @@ obj_desc_t *xref_reader_t::read_node(FILE *fp, obj_node_info_t &node) desc->type = static_cast(decode_uint32(p)); desc->fatal = (decode_uint8(p) != 0); memcpy(desc->name, p.read_bytes(name_len), name_len); + +// PAKSET_INFO("xref_reader_t::read_node()", "%s",desc->get_text() ); + return desc; }