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.
danalec
marked this pull request as ready for review
October 17, 2026 02:26
This was referenced Oct 20, 2026
danalec
added a commit
to danalec/Virtual-Display-Driver
that referenced
this pull request
Oct 20, 2026
…nging Two defects, both on paths that only fail under a race or under a rare condition, which is why they survived review. CoCreateGuid had its HRESULT discarded. MonitorInfo is zero-initialised, so a failed CoCreateGuid left MonitorContainerId as GUID_NULL, and that was then handed to IddCxMonitorCreate as if it were a valid container ID. The result is checked now, and on failure the driver logs and does not create that monitor. The frame loop's wait list is built conditionally: the buffer event goes in only when it is neither null nor INVALID_HANDLE_VALUE, and the terminate event only when its handle exists. The terminate event therefore has no fixed index. Testing WAIT_OBJECT_0 for "buffer available" first meant that a signalled terminate event at index 0 took the continue branch, and the thread re-entered IddCxSwapChainReleaseAndAcquireBuffer forever -- a hang in EvtDriverUnload. The wait result is decided by handle identity now. The invariant is that the loop exits in finitely many steps, not that it exits on the first wait. With both handles signalled, WaitForMultipleObjects returns the lower index, which is the buffer; the acquire resets that event, and the next wait returns terminate. Also here: the hardware cursor event is now closed by the driver. IddCx signals it but does not own it, and the driver used to leak the previous one on reassignment for the same monitor and the last one on unassign. s_KnownMonitorEdid is read under a lock now, because maincalc() reassigns it on another thread while CreateMonitor reads it. The EDID base block is 128 bytes and calculateChecksum() reads it unconditionally, so a short EDID was read past the end. AssignSwapChain can run on more than one thread, so its counter is atomic. And the device is validated before it is dereferenced: the check used to sit below the As() call, so a null m_Device was already dereferenced by the time it ran. Verified: Debug and Release on x64 and ARM64, 0 errors, all four producing MttVDD.dll, MttVDD.inf and a signed mttvdd.cat. Note: the two fixes that make the current master compile on a newer toolset (AvTaskHandle and CoCreateGuid) are in VirtualDrivers#519 and are not repeated here.
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 unchecked return values, both flagged by Code Analysis. On MSVC 18 these
stop the build; on the currently pinned CI toolset they do not surface at all,
so they have gone unnoticed. Neither is cosmetic.
1.
AvRevertMmThreadCharacteristicsreceives a handle that may beNULLAvSetMmThreadCharacteristicsWreturnsNULLon failure, and thatNULLwaspassed straight into
AvRevertMmThreadCharacteristics, which does not acceptit:
The revert now happens only when the handle is valid, with a log line for the
other case. When the characteristics were never set there is nothing to revert,
so the new branch is a log and no call.
2.
CoCreateGuidfailure is discardedMonitorInfois zero-initialised, so a failedCoCreateGuidleavesMonitorContainerIdasGUID_NULL, which is then handed toIddCxMonitorCreateas if it were a valid container ID. The result is nowchecked; on failure the driver logs and skips creating that monitor rather than
registering it under
GUID_NULL.Verification
Built Debug and Release on x64 and ARM64 — four configurations, each producing
MttVDD.dll,MttVDD.infand a signedmttvdd.cat, withTreatWarningAsErroron.
I found this by building a clean
origin/masterand finding it does notcompile on a newer toolset. Worth noting how it presents: the first
C2220aborts the compile before the next warning is emitted, so silencing
C28183does not produce a green build — it only moves the error to the next line. Both
had to be fixed to see whether anything else was behind them.
Not verified on hardware. No install, no monitor, no HDR toggle — this was a
build fix, and the change in behaviour is confined to two error paths that
previously did not exist.
If the two warnings do not appear on your CI toolset, that is expected — the
underlying defects are the same either way, and both are one-line guards.