2.6 KiB
Plan: Fix Undefined Behavior in DataTree Serialization
See also: RECOMMENDATIONS.md | PLAN.md | Architecture Overview
Current State
src/util/DataTree.h contains 20 reinterpret_cast operations:
- 18 read-side casts (in
get<T>()methods) — alias typed scalars throughunsigned charbuffers, violating strict aliasing rules (undefined behavior) - 2 write-side casts (in
set<T>()methods) — cast tounsigned char*, which is well-defined per [basic.lval]/11
The 18 read-side casts are the actual UB. The write-side casts are safe and idiomatic.
Implementation Plan
-
Create a helper template function in
DataTree.h(or a newDataTreeUtil.h):template <typename T> T readFromBuffer(const std::vector<unsigned char>& buf, size_t offset = 0) { T value; std::memcpy(&value, buf.data() + offset, sizeof(T)); return value; } template <typename T> void writeToBuffer(std::vector<unsigned char>& buf, const T& value) { const unsigned char* bytes = reinterpret_cast<const unsigned char*>(&value); buf.insert(buf.end(), bytes, bytes + sizeof(T)); }Note:
writeToBufferretains areinterpret_casttoconst unsigned char*. This cast is well-defined per [basic.lval]/11 — casting tounsigned char*(orstd::byte*) is the one exception to strict aliasing. It is safe and idiomatic. Only the read-side casts need replacement. -
Replace all 18
reinterpret_castread operations inget<T>()methods with calls toreadFromBuffer<T>(). -
Replace the 2
reinterpret_castwrite operations inset<T>()methods with calls towriteToBuffer<T>(). -
Add
#include <cstring>toDataTree.hforstd::memcpy.
Verification Criteria
- Zero
reinterpret_castread operations remain inDataTree.h(the 2 write-side casts tounsigned char*are expected). - Build succeeds on MSVC, GCC, and Clang with
-fstrict-aliasingenabled. - Existing serialization round-trip:
DataTree::SaveToFileXML()/LoadFromFileXML()still produces identical output for config and bookmark files. - No new compiler warnings under
-Wall -Wextra.
Rollback Strategy
This is a low-risk, localized change (single file). If regressions appear:
git revertthe commit.- The original
reinterpret_castcode is functionally equivalent on all major compilers despite being technically UB — the revert is safe.
Files to Modify
| File | Action |
|---|---|
src/util/DataTree.h |
Add helper functions, replace 18 read-side reinterpret_casts |