The compiler-warning sweep surfaced one real behavioral bug and a
handful of cosmetic noise:
REAL BUG — editor_app.cpp:1069 misleading-indentation:
if (objectPlacer_.objectCount() > 0 || npcSpawner_.spawnCount() > 0)
objectsDirty_ = true; autoSavePendingChanges_ = true;
The trailing `autoSavePendingChanges_ = true;` was OUTSIDE the if, so
the auto-save flag was set unconditionally on every reload — meaning
zones with zero objects/NPCs were getting needlessly auto-saved. Wrap
both writes in braces so they share the guard.
Cosmetic / noise — also fixed so the warning channel stays useful:
- cli_mesh_io.cpp:193,194,333 — same misleading-indentation pattern
(`if (a) x; if (b) y;` chained on one line). Replaced with std::clamp.
- cli_for_each.cpp:26,114,116 — `\\` at end of `//` comment was
treated as line-continuation, silently extending the comment to the
next line. Replaced the trailing-backslash convention with literal
"(continued)" markers in the example shell command.
- world_map/input_handler.cpp:14 — unused `cosmicEnabled` parameter
marked [[maybe_unused]].
- wowee_player_spawn_profiles.cpp:75 — unused CLS_PALADIN constant
marked [[maybe_unused]] (kept to document the bit layout).
- wowee_crafting_recipes.cpp — three unused `using R = …` aliases
removed.
- cli_data_tree.cpp:609, cli_format_validate.cpp:950 — unused argc
parameters marked [[maybe_unused]] in handlers I touched recently.
CMakeLists.txt: editor's CLI handlers all share `(int& i, int argc,
char** argv)` so they can plug into a function-pointer table; many
handlers don't reference argc. Added -Wno-unused-parameter to the
editor target rather than littering 30+ handler signatures with
[[maybe_unused]] noise. Main wowee target keeps the warning enabled.
Result: clean build with -Wall -Wextra -Wpedantic, zero warnings in
our code (the remaining ones are all in extern/ third-party headers).
CodeQL flagged 21 cpp/command-line-injection alerts in tools/editor/.
All matched the same pattern: build a shell command string from
argv[0] + a user-supplied path, then std::system() it. Even though
the threat model (user invokes their own CLI on their own machine)
makes the alert mostly academic, the std::system path is also
fragile — paths with spaces, quotes, or shell metacharacters
silently break.
Add tools/editor/cli_subprocess.{hpp,cpp} exposing a single
runChild(argv0, args, quiet=false) that uses posix_spawn on POSIX
and CreateProcess on Windows. No shell, argv passed verbatim,
optional stdout/stderr redirect to /dev/null (NUL on Windows).
Refactor 14 call sites across cli_convert.cpp, cli_data_tree.cpp,
cli_format_validate.cpp, cli_items.cpp, cli_random.cpp,
cli_repair.cpp, cli_spawn_audit.cpp.
Also fix two cpp/integer-multiplication-cast-to-long alerts:
- cli_gen_texture.cpp:3049 — seeds.reserve grid-size product
- cli_convert_single.cpp:224 — vector size for DBC record block
Both now widen one operand to size_t before multiplying.
Moves the seven proprietary-data-tree handlers out of main.cpp:
--migrate-data-tree --bench-migrate-data-tree
--list-data-tree-largest --export-data-tree-md
--info-data-tree --strip-data-tree
--audit-data-tree
All operate on a Blizzard-format extracted Data tree (the .m2/
.skin/.wmo/.blp/.dbc files) — they audit, migrate, or strip
proprietary-format files in support of the open-format
migration story.
Original placement spanned two sub-blocks (12546-12892 and
13093-13417 in main.cpp) interrupted by --gen-texture and
--add-texture-to-zone in the middle. Extraction collapses
both sub-blocks into one cohesive translation unit.
main.cpp drops 16,321 → 15,653 lines (-668). Behavior verified
by re-running --info-data-tree against a missing directory.
