@“Peter_Viscarola_(OSR)” said:
(two weeks later…)
b) It’s not even clear to me when stuff actually DOES apply to kernel-mode code, what that GETS me in real life or why on earth I’d want to use it. I’ll take Mr. Tippet’s example above:
NTSTATUS Example()
{
wil::unique_wdf_common_buffer my_buffer;
status = WdfCommonBufferCreate(..., &my_buffer);
if (STATUS_SUCCESS != status)
return status;
if (. . . something else fails . . . )
return STATUS_UNSUCCESSFUL; // no common buffer leak; WdfObjectDelete happens here
return STATUS_SUCCESS;
}
Common Buffers are typically long-lived – You allocate them during initialization, you parent them on your WDFDEVICE, they go away because the WDFDEVICE goes away (or, if you explicitly delete them just before your WDFDEVICE is destructed). So… Why would I want a Common Buffer deleted when leaving a function? I’m not picking on Common Buffer… but I see a whole LIST of similar structures: DMA Enabler, DMA Transaction, etc. These structures aren’t simply used within a function and returned… their entire POINT is to be created and used “across time” in your driver.
So… I feel like I must be missing something. Pray tell: What am I missing?
Peter
That may be a contrived example, but there are certainly a lot of things that should apply in kernel mode.
I didn’t spend a lot of time looking at the specifics what already exists, but it’s certainly a direction I think is positive.
In my previous place of work, we relied heavily on RAII wrappers for locks, mutexes and such. We also had C++ std-compatible memory wrappers (unique and shared ptr) and a kstd list, which reduced leaks and assured clear memory ownership. Once most system objects were wrapped behind c++ classes, we could provide usermode implementation and run unit tests on our logic.
If you look at the examples at https://github.com/microsoft/Network-Adapter-Class-Extension/tree/release_1903/rtl/inc you can see most objects have kernel as well as user mode implementations, which probably allows running the same logic in usermode too.
I see where you’re coming from regarding the difficulty of loving RETURN_IF_NTSTATUS_FAIL , and it is verbose and unusual.
However, there are certainly functions in my code where the entire function body is a dozen lines such as
status = Function(.....);
if (!NT_STATUS(status))
{
Log(.....,status);
goto failed;
}
status = Function2(.....);
if (!NT_STATUS(status))
{
Log(.....,status);
goto failed;
}
PVOID* ptr = ExAlloc(....)
if (!ptr)
{
Log(.....,status);
goto failed;
}
status = Function3(.....);
if (!NT_STATUS(status))
{
Log(.....,status);
goto failed;
}
and then the failed label has a long list of
if (thing)
Cleanup(thing);
if (thing2)
ObRelease(thing2);
if (ptr)
ExFreePool(ptr);
...
The use of similar macros, as well as RAII objects could reduce the amount of code spent on error handling, leading to
RETURN_ON_ERROR(Function(...));
RETURN_ON_ERROR(Function2(...));
PVOID* ptr = ExAlloc(....)
RETURN_IF_NULL(ptr);
RETURN_ON_ERROR(Function3(...));
Advantages would be:
- No cleanup / failed label. No leaked resources on error exit path.
- Code is short and you only see business logic (IDE syntax higlighting helps), and error handling is declarative, and unified
Obviously this style is very much a matter of taste.