Skip to content

Local engineering spec. Deferred to the 0.8.x modernization line; promote to a GitHub-primary issue + Project #2 entry before scheduling. Intentionally not RC2 work: every scope below is high-churn, low-bug-value, or regression-prone, which the minimum-drift stabilization rule keeps out of the release-candidate train.

REF-055 - Post-RC2 code-quality and C++ modernization backlog

Summary

A comprehensive code-quality assessment during the RC2 stabilization pass closed the small, bug-relevant, and low-risk items inline (targeted warning fixes, legacy-API bumps such as ChangeWindowMessageFilterEx/InitCommonControlsEx, removal of dead pre-Win10 scaffolding, and per-file C5219/C5033 suppressions for generated and stats/graph render code). What remains is genuine engineering debt that is valuable but too invasive for a stabilization train. This item records it as durable, prioritized scope for the 0.8.x modernization line.

Assessment evidence (2026-06-10)

Metrics from the emulebb-main worktree:

  • Manual memory management: ~1307 raw new / ~966 raw delete across the app, concentrated in EmuleDlg.cpp (80), PartFile.cpp (76), SharedFileList.cpp (62), BaseClient.cpp (55). Every lifetime defect fixed during this session (part-file/upload WakeUpCall cross-thread CList reads, the shutdown part-file UAF, the UPnP CString race) traced back to raw shared ownership without RAII or locking.
  • Type-unsafe casts: ~529 reinterpret_cast (much unavoidable: packet buffers, sockaddr, MFC messages) and ~171 const_cast (a smell that usually flags an ownership/const-correctness issue).
  • God-files (LOC): EmuleDlg.cpp 7239, PartFile.cpp 6670, SharedFileList.cpp 4934, WebServerJson.cpp 4648, WebServer.cpp 4643, Preferences.cpp 4465, OtherFunctions.cpp 4234, DownloadListCtrl.cpp 4067. High cyclomatic complexity (TryToConnect, FlushBuffer, OnInitDialog).
  • ~48 file-scope mutable statics — global state with thread-safety and testability friction.
  • Warning audit: after the inline fixes plus the stats-display C5219 suppression, a full build is dominated by benign MFC message-map fn-ptr casts (C4191 ~84) plus a thin tail; no further high-value warning work remains.

Scope (prioritized)

  1. RAII / smart-pointer migration of the lifetime-critical raw new/delete (highest value). Target the I/O and client-lifetime paths first (PartFile, the IOCP write/upload threads, BaseClient/DownloadClient, SharedFileList). This is the structural fix that prevents the exact UAF/cross-thread-race class patched piecemeal during RC2 hardening. Must preserve eD2K/Kad wire semantics and the existing seam test points.
  2. Decompose the god-files. Split EmuleDlg.cpp, PartFile.cpp, and SharedFileList.cpp along clear responsibility seams. High maintainability value, high regression risk; do behind tests, incrementally.
  3. const_cast audit (~171 sites). Likely surfaces a few latent ownership and const-correctness bugs; exploratory.
  4. WM_THEMECHANGED migration. ButtonsTabCtrl.cpp, ClosableTabCtrl.cpp, and DialogMinTrayBtn.cpp hand-roll _WM_THEMECHANGED / _ON_WM_THEMECHANGED ("XP only"); the Win10 SDK + MFC now provide WM_THEMECHANGED / ON_WM_THEMECHANGED. Signature-sensitive swap; delete the stale "XP only" comments at the same time.
  5. Statistics.cpp C5219 per-site review (~24). Unlike the pure-display files (suppressed per-file), Statistics.cpp is the data layer; review each uint64->double conversion individually rather than blanket-disabling.
  6. C5038 init-list order (PPgTweaks, SearchFile). Benign (members init in declaration order regardless). A clean fix needs reordering whole init-list segments to match declaration order, not single swaps; do during the relevant class refactor, not standalone.

Out of scope / tracked elsewhere

  • The C4191 MFC message-map casts (~84) are a standard framework pattern, not bugs — leave.
  • Dependency currency (cryptopp 8.4.0 -> 8.9.0; nlohmann-json 3.11.3 -> 3.12.0; the abandoned id3lib parsing untrusted ID3 tags) is build-topology work owned by emulebb-build, tracked separately from this source-refactor item.

Acceptance criteria

  • No behavioral drift at the eD2K/Kad protocol layer.
  • Each scope lands as its own reviewed, test-covered change on the 0.8.x line.
  • Full-matrix validation for any change that touches the build contract or broad integration surface.