Skip to content

Workflow status is tracked in GitHub: https://github.com/emulebb/emulebb/issues/47. This local document is retained as an engineering spec/evidence record.

CI-010 - Reduce remaining app-local warning debt after external noise cleanup

Summary

The first warning-noise reduction pass already contained SDK and third-party header spam and cut the Release|x64 warning floor from 5092 to 828. What remains is now mostly app-local warning debt from srchybrid, and that debt should be tracked separately from the deferred REF-021 Winsock cleanup.

This item is about the remaining local warnings that are still worth fixing in our own code, plus explicit triage of the buckets that are probably framework noise rather than real correctness work.

Current State

Original reference build:

  • EMULEBB_WORKSPACE_ROOT\workspaces\workspace\state\build-logs\20260419-172533

Original top remaining warning buckets from Release|x64:

  • C5219: 464
  • C4191: 96
  • C4244: 52
  • C5262: 44
  • C5033: 34
  • C4311: 30

Representative remaining local hotspots:

  • srchybrid/StatisticsDlg.cpp — C5219 (114)
  • srchybrid/PartFile.cpp — C5219 (14)
  • srchybrid/Statistics.cpp — C5219 (12)
  • srchybrid/EmuleDlg.cpp — C5219 (11)
  • srchybrid/FileDetailDialogInfo.cpp — C5219 (8)
  • srchybrid/UploadQueue.cpp — C5219 (6)
  • srchybrid/ServerConnect.cpp — C4244 (5)
  • srchybrid/SearchListCtrl.cpp — C5219 (5), C5262 (4)
  • srchybrid/DownloadListCtrl.cpp — C4244 (3), C5262 (2)

Updated reference build:

  • EMULEBB_WORKSPACE_ROOT\workspaces\workspace\state\build-logs\20260502-112930
  • command: build-app --config Release --platform x64 --variant main
  • result: build succeeded; APP main reported 862 warnings in the workspace summary, while the raw MSBuild footer reported 391 Warning(s)
  • de-duplicated located compiler warning sites in the app log: 379
  • distinct command-line warning families: 2
  • distinct linker-warning lines: 58

Current located compiler warning buckets from Release|x64:

  • C5219
  • Count: 225

  • Practical impact: Float-to-integral conversion after arithmetic conversion. Usually UI/stat precision debt, but it can matter in transfer limits, progress, scheduling, scoring, and persisted values.

  • C4191

  • Count: 48

  • Practical impact: Unsafe function-pointer casts. Many are likely MFC message-map plumbing, but any handler signature mismatch would become a real call-convention/runtime bug. Needs triage, not blanket removal.

  • C4244

  • Count: 26

  • Practical impact: Narrowing conversions. Highest value when source width is semantically larger, for example GetTickCount64() into DWORD, sizes, offsets, and rates.

  • C5262

  • Count: 18

  • Practical impact: Implicit fallthrough. Some sites are intentional UI flow, but the compiler cannot distinguish that from missed break logic.

  • C5033

  • Count: 17

  • Practical impact: register is no longer supported. Cosmetic/source-age cleanup unless it blocks a stricter toolchain.

  • C4311

  • Count: 15

  • Practical impact: Pointer or handle truncation to smaller integer type. High x64 risk if the truncated value is used as an identifier, menu handle, window data, or callback payload.

  • C4312

  • Count: 7

  • Practical impact: Conversion from smaller integer to pointer-sized type. Often paired with legacy Win32/MFC message packing and should be audited with adjacent C4311/C4302.

  • C4302

  • Count: 6

  • Practical impact: Truncation from pointer-sized value to smaller type. High x64 risk for handles and pointer cookies.

  • C5038

  • Count: 4

  • Practical impact: Member initializer order mismatch. Usually not a bug when initializers are independent, but real if later members depend on earlier initialization side effects.

  • C5245

  • Count: 4

  • Practical impact: Unreferenced internal function removed. Low runtime risk; useful dead-code signal.

  • C4840

  • Count: 3

  • Practical impact: Non-portable variadic argument use. Real type-safety issue when passing C++ objects such as CString through C varargs.

  • C5039

  • Count: 3

  • Practical impact: Potential throwing callback passed to C/Win32 API. Real stability risk if an exception crosses a C ABI callback boundary.

  • C4100

  • Count: 2

  • Practical impact: Unreferenced formal parameter. Low risk; can be intentional virtual/API compatibility.

  • C4266

  • Count: 1
  • Practical impact: Hidden virtual override. Can be real behavior drift if the intended override signature no longer matches the base.

Current top files by de-duplicated located compiler warnings:

File Count Dominant concern
srchybrid/StatisticsDlg.cpp 116 Mostly C5219; UI/stat numeric intent.
srchybrid/Scanner.cpp 18 Mixed local warning debt.
srchybrid/DownloadListCtrl.cpp 17 Pointer/handle casts plus fallthrough and narrowing.
srchybrid/Statistics.cpp 16 Mostly numeric precision/conversion debt.
srchybrid/PartFile.cpp 16 Transfer/file numeric conversion debt.
srchybrid/EmuleDlg.cpp 15 Mixed UI/control-flow and initialization warnings.
srchybrid/Emule.cpp 9 Initializer-order and legacy command-line override warnings.
srchybrid/SearchListCtrl.cpp 9 UI fallthrough/numeric warning debt.
srchybrid/FileDetailDialogInfo.cpp 8 UI/detail numeric conversion debt.
srchybrid/BarShader.cpp 7 Drawing/math conversion debt.
srchybrid/UploadQueue.cpp 7 Queue/rate numeric conversion debt.

Other notable files with 5 warnings each: ListenSocket.cpp, MediaInfo.cpp, ServerConnect.cpp, SharedDirsTreeCtrl.cpp, and SharedFilesCtrl.cpp. Files with 4 warnings each include UPnPImplPcpNatPmp.cpp, KadLookupGraph.cpp, OtherFunctions.cpp, and SearchList.cpp.

Suppression Context

The active project intentionally runs at a high warning level:

  • srchybrid\emule.vcxproj uses WarningLevel=EnableAllWarnings
  • srchybrid\emule.vcxproj keeps external headers quiet with ExternalWarningLevel=TurnOffAllWarnings
  • srchybrid\emule.vcxproj still marks external includes with /external:anglebrackets and /external:I$(ThirdPartyRoot) to keep third-party and SDK header mass out of the local warning floor

The 2026-05-02 warning audit found repeated D9025 command-line warnings because MSBuild derived /external:W4 from EnableAllWarnings, then the project appended raw /external:W0 in AdditionalOptions. This was build configuration noise, not source warning debt. The fix is to express the desired external-header policy through ExternalWarningLevel=TurnOffAllWarnings and keep raw /external:W* switches out of AdditionalOptions.

The broad warning suppressions in srchybrid\Stdafx.h:43-69 are therefore not the only reason warning debt exists; many warnings still surface even with those suppression pragmas in place. The suppressions currently hide additional families that should not be removed blindly:

  • hidden switch-enum coverage: 4061, 4062
  • unsafe function-pointer casts: 4191
  • hidden or mismatched virtual overrides: 4263, 4264, 4265, 4266
  • signed/unsigned conversions: 4365
  • object layout, generated special member, and padding noise: 4435, 4625, 4626, 4820, 5026, 5027
  • exception and local-static behavior changes: 4571, 4640
  • preprocessor and inline/noise diagnostics: 4619, 4668, 4710, 4711
  • performance/Spectre/volatile diagnostics: 4738, 4746, 5045, 5220
  • GUID attribute noise: 4917

The same file still defines _CRT_SECURE_NO_DEPRECATE and _WINSOCK_DEPRECATED_NO_WARNINGS, and emule.vcxproj still injects _CRT_SECURE_NO_DEPRECATE as a project-wide preprocessor definition. Those are owned by REF-021; this item tracks the active app-local warning floor that is visible even before removing those deprecation suppressions.

Actual Impact Notes

Highest value source fixes:

  • C4244, C4311, C4302, and C4312 are the strongest correctness signals because they can truncate time values, sizes, pointers, or Win32 handles on x64.
  • C5262 should be reviewed before changing logic. Add explicit [[fallthrough]] only where existing fallthrough is intentional; otherwise add the missing break or equivalent control flow.
  • C4840 should be fixed locally with explicit LPCTSTR extraction or type-safe formatting instead of passing C++ objects through varargs.
  • C5038 should be fixed where constructor initializer order obscures dependencies; this is low-cost and removes a class of future maintenance mistakes.
  • C5039 should be fixed by making callbacks noexcept or wrapping exception boundaries before handing function pointers to C/Win32 APIs.
  • C4266 is a single site but deserves review because hidden virtual functions can silently change dispatch behavior.

Lower value cleanup:

  • C5033 and C4100 are mostly source-age or interface-noise cleanup.
  • C5245 points at removable dead internal functions; fix only when touching adjacent code unless warning-floor work is explicitly prioritized.
  • Most C5219 in display/statistics code is likely low-to-medium runtime risk, but C5219 in scheduling, scoring, transfer limits, byte counts, or persisted values should be audited before being dismissed as cosmetic.

Current concrete examples from the 2026-05-02 log:

  • srchybrid\ClientUDPSocket.cpp:439 assigns GetTickCount64() into DWORD. This can reintroduce 32-bit tick wrap behavior in UDP queue timing logic.
  • srchybrid\DownloadListCtrl.cpp:1073, 1096, 1203, and 1211 cast menu handles through UINT. On x64 this is handle truncation and should be replaced with pointer-width-safe types.
  • srchybrid\kademlia\kademlia\Search.cpp:1242-1245 passes uint32 values as LPCTSTR varargs payloads. This appears to be legacy typed-varargs packing and needs a careful local audit rather than a mechanical cast change.
  • srchybrid\DownloadListCtrl.cpp:831 and 1414 are representative C5262 fallthroughs. Some may be intended UI command grouping; annotate or split only after reading the surrounding switch.
  • srchybrid\UPnPImplPcpNatPmp.cpp:248, 306, and 308 pass CString into variadic calls. These should use explicit (LPCTSTR) or a type-safe helper.
  • srchybrid\Emule.cpp:796, 797, and 808, plus srchybrid\PPgTweaks.cpp:641, show C5038 initializer-order drift.
  • srchybrid\OtherFunctions.cpp:3368, OtherFunctions.cpp:3383, and srchybrid\TreeOptionsCtrl.cpp:1941 pass callbacks to C/Win32 APIs where a throwing callback would cross an ABI boundary.
  • srchybrid\Emule.cpp:630 is the current C4266 hidden-virtual site for a CCommandLineInfo::ParseParam overload.

Linker warning impact:

  • LNK4286 warnings were mostly pcpnatpmp.lib importing UCRT symbols.
  • LNK4098 reported a default-library conflict with MSVCRT.
  • Root cause: libpcpnatpmp was configured by CMake with MSVC's dynamic CRT defaults (/MD and /MDd) while the app policy requires the static CRT (/MT and /MTd).
  • Fix direction: handle this in build orchestration by passing CMAKE_POLICY_DEFAULT_CMP0091=NEW and CMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded$<$<CONFIG:Debug>:Debug> when configuring libpcpnatpmp. Do not hide the conflict with /NODEFAULTLIB:MSVCRT.
  • Verified fix: clean libpcpnatpmp rebuilds now generate pcpnatpmp.vcxproj with MultiThreadedDebug/MultiThreaded runtime settings for x64 and ARM64, and the active app links no longer emit LNK4098 or LNK4286 in the checked Debug/Release x64 and ARM64 logs.

Progress

Iterative CI-010 slices have landed on main, cutting the located Release|x64 compiler-warning floor from 828 toward the high-300s. The landed work removed local C5039, C4244, C4311, C4302, C4312, C4840, C5038, C5245, C5262, and C4266 sites across the download, shared-files, search, part-file, server/upload, queue, and IRC paths, each with scoped Debug/Release x64 revalidation.

The item stays IN_PROGRESS: the remaining local debt is the separately triaged C4191 MFC message-map bucket and the high-volume C5219 numeric/precision clusters in statistics, part-file, and UI code.

The full commit-by-commit landed-work and scoped-validation trail is preserved in the CI-010 warning cleanup progress log.

Triage Split

Real source corrections to make later

C5262 implicit fallthroughs

These are control-flow ambiguities and should be fixed or marked explicitly.

Representative sites:

  • srchybrid/DownloadListCtrl.cpp:754
  • srchybrid/DownloadListCtrl.cpp:1335
  • srchybrid/SearchListCtrl.cpp:1303
  • srchybrid/SearchListCtrl.cpp:1429
  • srchybrid/SearchListCtrl.cpp:1431
  • srchybrid/SearchListCtrl.cpp:1448

C4244 integer narrowing that may truncate

These should be audited for real range intent rather than cast away blindly.

Representative sites:

  • srchybrid/DownloadListCtrl.cpp:444
  • srchybrid/EmuleDlg.cpp:3117
  • srchybrid/UploadQueue.cpp:89
  • srchybrid/ServerConnect.cpp (multiple current sites)

Targeted C5219 math/precision cleanup

These are often not hard bugs, but in transfer, stats, and progress code they should be made explicit where the numeric intent is underspecified.

Highest-yield current clusters:

  • srchybrid/StatisticsDlg.cpp
  • srchybrid/PartFile.cpp
  • srchybrid/Statistics.cpp
  • srchybrid/EmuleDlg.cpp
  • srchybrid/FileDetailDialogInfo.cpp
  • srchybrid/UploadQueue.cpp
  • srchybrid/kademlia/kademlia/Prefs.cpp

Buckets that need triage before changing code

C4191 MFC/message-map casts

These are likely framework-era plumbing noise rather than automatic bug fixes. Do not treat them as mandatory behavior changes until each site is reviewed.

Representative current sites:

  • srchybrid/DownloadListCtrl.cpp:72
  • srchybrid/EmuleDlg.cpp:236
  • srchybrid/KadSearchListCtrl.cpp:42

Default direction:

  • prefer narrow suppression or framework-aware triage where appropriate
  • do not churn stable MFC message-map plumbing just to force the warning count down

Out Of Scope

  • REF-021 deprecated Winsock API cleanup and blanket warning-suppression removal remain explicitly deferred and are not part of this item.
  • SDK, MFC, ATL, and third-party header warning mass is already treated separately by the external-header cleanup and should not be reopened here.

Acceptance Criteria

  • [ ] Remaining C5262 fallthrough sites in active local UI/transfer paths are fixed or explicitly annotated
  • [ ] High-risk C4244 local narrowing sites are reviewed and corrected where truncation is real
  • [ ] The highest-volume local C5219 clusters are reduced with explicit numeric intent
  • [ ] C4191 sites are triaged separately instead of being mixed into blind warning-count cleanup
  • [ ] Release|x64 warning totals drop further without re-expanding external-header noise or reviving deferred REF-021 work

2026-04-19 Decision

This warning-cleanup follow-through was initially deferred in favor of more immediately useful runtime work. It has since reopened for narrow, low-drift warning reductions that fall out of 0.7.3 RC1 hardening and validation work.