Wider range of check return values
I'd like to overhaul the way we run checks and standardise how we handle errors. Right now we just return true or false, which isn't very useful.
What I propose is the following:
First we replace BOOL return values with an enum of {good, bad, warn, fail}. Good means the check completed successfully and nothing was detected. Bad means the check completed successfully and detected something. Warn means the outcome of the check was inconclusive. Fail means that some prerequisite was not met or an API call failed.
Secondly we add a standardised mechanism for reporting additional information from checks that doesn't mess up the output format. Effectively we offer a printf equivalent function for output that pushes messages to an std::vector which we can then print out after the check completes. I'd like to also have these messages tagged with some sort of level so that we can filter them - by default only showing errors (so we can say why a check failed if it failed), but with the ability to specify a debug level on the command line at a later date for increased verbosity.
@LordNoteworthy I'd like your feedback on this before I go ahead and start the work.
Hello @gsuberland.
Dead right, atm we have no differentiation between a failed API call and when nothing get detected. We have to implement something like that, that would help us to catch potential issues and easily figure out community issues.
I am also in about the mechanism for reporting additional information. As far as I am concerned, you can go ahead with this feature.
Cool, I'll probably start on this today.
Only took me three years to actually get started. Oh dear.
I'm working on this now. Every check will now return a CheckResult object that contains a more granular status, a message, and log information.
Statuses include:
- Detected
- Undetected
- Error
- Invalid
- Inconclusive
Detected, undetected, and error are pretty self-explanatory. Invalid is returned when a check cannot be completed because there's some missing API or the check simply doesn't exist on the system (e.g. 32-bit check on x64). Inconclusive is used (rarely) with checks that might be able to detect or rule out the presence of a debugger/VM but for whatever reason couldn't do either.
I haven't bothered with log levels. Instead each status has a message and a log. The message should be shown in verbose output, whereas the log should be shown in debug output.
There are some macros to help with this so you don't have to write return CheckResult::CheckResult(CheckStatus::Detected); all over the place, e.g. RESULT_DETECTED, RESULT_UNDETECTED, and DETECTED_IF_TRUE.
Example of some of the new code:
#include "pch.h"
#include "WUDF_IsDebuggerPresent.h"
CheckResult WUDF_IsAnyDebuggerPresent()
{
if (API::IsAvailable(API_IDENTIFIER::API_WudfIsAnyDebuggerPresent))
{
auto WudfIsAnyDebuggerPresent = static_cast<pWudfIsAnyDebuggerPresent>(API::GetAPI(API_IDENTIFIER::API_WudfIsAnyDebuggerPresent));
return DETECTED_IF_FALSE(WudfIsAnyDebuggerPresent() == 0);
}
else
return CheckResult::Invalid("WudfIsAnyDebuggerPresent is not available on this system. Check cannot be performed.");
}
CheckResult WUDF_IsKernelDebuggerPresent()
{
if (API::IsAvailable(API_IDENTIFIER::API_WudfIsKernelDebuggerPresent))
{
auto WudfIsKernelDebuggerPresent = static_cast<pWudfIsKernelDebuggerPresent>(API::GetAPI(API_IDENTIFIER::API_WudfIsKernelDebuggerPresent));
return DETECTED_IF_FALSE(WudfIsKernelDebuggerPresent() == 0);
}
else
return CheckResult::Invalid("WudfIsKernelDebuggerPresent is not available on this system. Check cannot be performed.");
}
CheckResult WUDF_IsUserDebuggerPresent()
{
if (API::IsAvailable(API_IDENTIFIER::API_WudfIsUserDebuggerPresent))
{
auto WudfIsUserDebuggerPresent = static_cast<pWudfIsKernelDebuggerPresent>(API::GetAPI(API_IDENTIFIER::API_WudfIsUserDebuggerPresent));
return DETECTED_IF_FALSE(WudfIsUserDebuggerPresent() == 0);
}
else
return CheckResult::Invalid("WudfIsUserDebuggerPresent is not available on this system. Check cannot be performed.");
}
Good design ! looks all good.