Repository navigation
Conversation
Two unchecked return values. On MSVC 18 both stop the build; on the pinned CI toolset neither surfaces. Neither defect is cosmetic. AvSetMmThreadCharacteristicsW returns NULL on failure, and that NULL went straight into AvRevertMmThreadCharacteristics, which does not accept it. Reported as C28183 and promoted to C2220 by TreatWarningAsError. The revert now happens only when the handle is valid; when the characteristics were never set there is nothing to revert, so that branch is a log and no call. CoCreateGuid had its HRESULT discarded (C6031 -> C2220). MonitorInfo is zero-initialised, so a failure left MonitorContainerId as GUID_NULL, which was then handed to IddCxMonitorCreate as if it were valid. The result is now checked, and on failure the driver logs and skips creating that monitor rather than registering it under GUID_NULL. Note how this presents: the first C2220 aborts the compile before the next warning is emitted, so silencing C28183 does not give a green build, it only moves the error to the next line. Both had to be fixed to find out whether anything was behind them. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat.
Two things the rest of the driver builds on: the shared state is actually guarded, and there is a way to parse a user-supplied number that does not throw out of a UMDF callback. No behaviour change. g_Running becomes std::atomic<bool>: the pipe server thread flips it from a command while the main path reads it. g_pipeHandle is only written through SetPipeHandle()/ClearPipeHandle(), under a mutex that is deliberately not g_Mutex. vddlog() is called from inside the pipe server while that thread holds g_Mutex, so SendToPipe() cannot take g_Mutex without deadlocking on it. ClearPipeHandle() returns the previous value so the caller can close it, and the handle is withdrawn before the close. s_KnownMonitorModesMutex protects a std::vector that two IddCx callbacks can rewrite at the same time -- one clears and refills it while the other indexes it. That is a live data race, not a stale read. Readers take a snapshot under the lock instead of holding it across the IddCx calls that follow. g_MetadataMutex guards the HDR and gamma stores, which the settings loader and the IddCx callbacks both write. vdd_settings.xml, option.txt and monitor_profile.xml are user-editable, and loadSettings() runs inside the DeviceAdd callback. std::stoi and std::stof throw on a malformed value, and an exception leaving a UMDF callback crashes the host. ParseIntSafe, ParseFloatSafe and ParseDoubleSafe log the offending value and return the caller's fallback instead, and they reject trailing garbage. ParseDoubleSafe exists because the EDID chromaticity fields are double: redX 0.640 read as float and stored back as double is 0.63999998569488525390625, which then differs from the 0.640 the same file computes elsewhere. ClampToUINT16 exists because a light level above 65535 wrapped silently on the cast. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat.
This was referenced Oct 20, 2026
Author
|
Closing this without merging. The work is real and it is verified -- every branch builds clean in Debug and The code stays available on the fork, split into reviewable pieces: fix/code-analysis-errors 1 file +14/-2 If any of it is ever wanted, the branches are there and this PR can be |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two things that the rest of the driver builds on: the shared state is actually
guarded, and there is a way to parse a user-supplied number that does not throw
out of a UMDF callback.
One file, +177 / −5. No behaviour change — the guarded paths were already
unreachable in the normal case.
Shared state
g_Runningis nowstd::atomic<bool>. The pipe server thread flips itfrom a command while the main path reads it, and it was a plain
bool.g_pipeHandleis only written throughSetPipeHandle()/ClearPipeHandle(),under a dedicated mutex. That mutex is deliberately not
g_Mutex:vddlog()is called from inside the pipe server while that thread holdsg_Mutex, soSendToPipe()cannot takeg_Mutexwithout deadlocking on it.ClearPipeHandle()returns the previous value so the caller can close it, andthe handle is withdrawn before the close — otherwise another thread could pick
up a handle that is already being closed.
s_KnownMonitorModesMutexprotects astd::vectorthat two IddCx callbackscan rewrite at the same time.
VirtualDisplayDriverParseMonitorDescriptionand
...2both callRebuildKnownMonitorModesCache(), and one can clear andrefill the vector while the other is indexing it. That is a live data race on
a
std::vector— undefined behaviour, not a stale read. Readers now take asnapshot under the lock instead of holding it across the IddCx calls that
follow.
g_MetadataMutexguards the HDR and gamma stores, which are written by thesettings loader and by the IddCx callbacks.
std::mapis not safe to readwhile another thread inserts. Again distinct from
g_Mutex, because thosecallbacks log along the way.
Checked parsing
vdd_settings.xml,option.txtandmonitor_profile.xmlare user-editablefiles, and
loadSettings()runs inside theDeviceAddcallback.std::stoiand
std::stofthrowstd::invalid_argumentorstd::out_of_rangeon amalformed value, and an exception leaving a UMDF callback crashes the host.
ParseIntSafe,ParseFloatSafeandParseDoubleSafelog the offendingvalue and return the caller's fallback instead. They also reject trailing
garbage —
" 480 "parses as 480 withstoi, but the trailing space iscounted as unconsumed input, so it is reported rather than silently accepted.
ParseDoubleSafeexists because the EDID profile's chromaticity andluminance fields are
double.ParseFloatSafewould quietly round them:redX0.640 read asfloatand stored back asdoubleis0.63999998569488525390625, which then differs from the 0.640 the same file
computes elsewhere.
ClampToUINT16exists because a light level above 65535 wrapped silently onthe cast, turning a typo in the XML into a plausible-looking wrong number.
Verification
Debug and Release on x64 and ARM64 — four configurations, each producing
MttVDD.dll,MttVDD.infand a signedmttvdd.cat, withTreatWarningAsErroron.No hardware was involved: this changes how shared state is reached, not what
the driver does once it is reached.