Compile with /W3 on Windows#45
Conversation
Enable /WX (treat warnings as errors). Fix some basic errors that were found. Enable /FC (emit full path on diagnostics) to make it easier for IDE. Add (commented out) entries for enabling debug builds for dev.
|
Troubling... Check failed on Windows but it succeeded locally. Will investigate. |
|
Updated local env to Python 3.9 to uncover new UNICODE deprecation warnings, which now trigger errors due to /WX! |
| { | ||
| return -(S_FALSE); | ||
| } | ||
| //UNUSED? if (!CheckWindowsSharedMemoryPrerequisites(pMappingName)) |
There was a problem hiding this comment.
Why are you considering the CheckWindowsSharedMemoryPrerequisites() function call as unused here?
There was a problem hiding this comment.
Because in this UtilMappedViewReadBegin() the pMappingName is uninitialized, checked and subsequently unused. Likely the result of large copy+paste. I don't quite know where this is used, if it is, so I opted for the loud, ugly commenting-out style of remediation.
There was a problem hiding this comment.
In this case, it looks like the check is just happening in the wrong place; it looks like L567 is probably a better place for it (just prior to when the shared-memory mapping is created). If the check fails, you'd also need to dispose the allocated file handle before returning -(S_FALSE). The purpose of the check is to better handle the case where the user doesn't have permissions to create the shared-memory mapping -- so we can provide a reasonable error message to the user; otherwise, the call just fails and it's very non-obvious to the end-user why it happened.
There was a problem hiding this comment.
I thought so too, until I saw on line 578 that pMappingName is unused. Hence none of this is necessary.
I don't know offhand if pMappingName should be used, in which case I would unwind all this UNUSED? nonsense and complete the implementation properly.
There was a problem hiding this comment.
I created a new issue for implementing this and will keep the code commented out so it's safe.
2 participants
Enable /WX (treat warnings as errors).
Fix some basic errors that were found.
Enable /FC (emit full path on diagnostics) to make it easier for IDE.
Add (commented out) entries for enabling debug builds for dev.