Windows System Software -- Consulting, Training, Development -- Unique Expertise, Guaranteed Results

Before Posting...
Please check out the Community Guidelines in the Announcements and Administration Category.

The new wil library for C++ code in drivers

Jeffrey_Tippet_[MSFT]Jeffrey_Tippet_[MSFT] Member - All Emails Posts: 513
edited May 11 in NTDEV

If you write your driver in C++, I'd like to draw your attention to one of the less visible outputs of the Microsoft Build 2019 conference.

We released a library called "wil", which you can get here: https://github.com/microsoft/wil

I can't articulate any single unifying theme of the library; it's just a collection of useful little wrappers. It won't change your life, but it might mean you can avoid a few memory leaks.

wil was primarily developed by the Windows shell team for writing usermode code, but the NDIS and Bluetooth teams have contributed some small kernel-specific features. Here's a few examples of how you can use wil in your kernel driver:

Memory leaks

The wil::unique_any type works like std::unique_ptr, except instead of managing a block of memory, it manages a handle to something. It's designed to be specialized for each type of handle in the OS. So for example, wil::unique_wdf_common_buffer works as a wrapper around a WDFCOMMONBUFFER handle. It automatically calls WdfObjectDelete when it goes out of scope.

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;
}

(Note that wil::out_param can clean up that example even further; C++ is working on standardizing that: https://wg21.link/p1132 .)

You're encouraged to add your own specializations. So for example, wil doesn't currently have one for NDIS NBL pools, but you can create one by adding 1 line to your code:

using unique_nbl_pool = wil::unique_any<NDIS_HANDLE, decltype(&::NdisFreeNetBufferListPool), &::NdisFreeNetBufferListPool>;

This creates a type called unique_nbl_pool that holds an NDIS_HANDLE, and will automatically call NdisFreeNetBufferListPool when it goes out of scope, if the handle value is not nullptr.

wil includes a scope guard. (C++ will get one soon: https://wg21.link/p0052 ) This is a common thing for people to want; you may already have your own scope guard. But if you don't have one, well now you do:

NTSTATUS Example()
{
    my_acquire(&lock);
    lock->owner = KeGetCurrentThread();

    // everything in this lambda runs when 'guard' goes out of scope
    auto guard = wil::scope_exit([&]() {
        lock->owner = nullptr;
        release(&lock);
    });

     if ( . . . failure . . .)
         return STATUS_UNSUCCESSFUL; // lock is released

     return STATUS_SUCCESS;
}

A subset of the STL

Microsoft's toolchain does not ship a copy of the STL that works in kernel mode. Partly this is because the kernel's CRT doesn't support C++ exceptions. (And partly this is because I/O is wildly different in kernel, so you'd have to rewrite the implementation of all the I/O libraries.)

But for kernel developers, wil ships a subset of an STL implementation. To avoid conflicting with the real STL, it's available under the wistd namespace. The rule of thumb is that wistd::foo is a drop-in replacement for std::foo.

This subset contains type_traits, so you can do wistd::move, wistd::is_pointer, or even wistd::is_trivially_destructible_v.

The library also provides a clone of std::unique_ptr. So now you can do:

using unique_nbl = wistd::unique_ptr<NET_BUFFER_LIST, wil::function_deleter<decltype(&NdisFreeNetBufferList), NdisFreeNetBufferList>>;

(Note also the use of wil::function_deleter, to make it slightly more convenient to provide a custom delete functor to unique_ptr. I think there's some effort to push this into the real STL.)

FAQ: what's the difference between this use of unique_ptr and the prior example of unique_any? unique_ptr holds a block of memory that you want to reach into. So it makes sense to say nbl->Status = x;. unique_any holds a handle value that you can't dereference directly. You can't say `nblPool->???? = ???;'.

Locking

This version of wil has an early cut of our thinking around a safe lock wrapper around KSPIN_LOCK, named wil::acquire_kspin_lock. It's best explained with an example:

KSPIN_LOCK MyGlobalLock;
NTSTATUS Example()
{
    auto lock = wil::acquire_kspin_lock(&MyGlobalLock);

    if (. . . something . . .)
         return STATUS_UNSUCCESSFUL;  // lock is automatically released

    return STATUS_SUCCESS; // lock is automatically released
}

Note that, in addition to preventing a leaked lock, you also don't have to fuss with OldIrql. It's still there, and you're still raising to DISPATCH_LEVEL like before, but now it's happening automatically. (Whether you think this is a bug or a feature is up to your tastes. If you don't like RAII types that hide IRQL changes, then don't use wil::acquire_kspin_lock. For what it's worth, I've been tidying up the implementation of NDIS.sys by hiding a lot of these sorts of mechanics, and I prefer to see only high-level code. But I am aware that there are many excellent developers who want to explicitly see every meaningful state change, including IRQL changes.)

Future

First off, let me point out that this library is used to implement large parts of the OS. There are hundreds of developers here who use it. So unlike, uh, some other things that get tossed onto github, this project is not likely to wither and die tomorrow.

There are, however, only a handful of kernel developers working on the library, so the kernel support has been coming along much slower. I'd like to expand the existing kernel features in depth (more builtin handle types, more types of locks), but also depth (wrappers for KEVENT and LIST_ENTRY, help for paging code, a story for operator new, generic atomics). I've also had to admit failure on some experiments in wrapping UNICODE_STRING and WORK_QUEUE_ITEM, but would love to see someone smarter than me solve these.

One of the most valuable parts of the library -- error checking & logging -- doesn't work at all in kernel mode. I am sure it's possible; we just need someone to sort it out. (I'm not trying to hint that you should do it; we own this and will get to it eventually. But if you do figure it out, we do accept pull requests!) This sort of stuff ends up being really useful in C++ code, since you can get a nice log of where the failures are, without cluttering your code with a bunch of printfs.

NTSTATUS Example()
{
    RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));
    RETURN_IF_NTSTATUS_FAILED_MSG(ZwWriteFile(. . . .), "Failed to write %u bytes", size);
    FAIL_FAST_IF_NULL(handle); // bugcheck
    LOG_IF_NTSTATUS_FAILED(ZwClose(. . . ));
    return STATUS_SUCCESS;
}

Anyway, this code's another tool for you to write drivers with. Give it a try if you like. If something is broken, open an issue on github or try and catch my attention here. We want it to be useful for people outside of the Windows team too.

Comments

  • RourkeRourke Member Posts: 31

    the NDIS and Bluetooth teams have contributed some small kernel-specific features

    Thanks for sharing and taking the time to write out such an in depth presentation. It is interesting to learn of. From my perspective it sounds somewhat useful, but not compelling enough to pull in a proprietary library just for this. Part of this assessment is some of it is trivial to roll one's own and I already have done so. The functionality also seems light for what is implemented. For example, no object for KSPIN_LOCK and thus no guaranteed initialization. I am sure some people will like it though.

    Microsoft's toolchain does not ship a copy of the STL that works in kernel mode

    The WDK is using standard library files that are older than some people here. What is preventing getting these updated to the latest file set? The language has marched a long way since the 90's. No one is asking for exceptions and i/o. But certainly std::byte, std::array, std::unique_ptr, and others would be terrific to have in the kernel arsenal. The key here is to allow use of standard things (std::) that are well documented and understood by developers, not proprietary wil:: things copy and pasted from std:: concepts. And the foundation of any higher level c++ library should be built on std:: building blocks.

  • Dejan_MaksimovicDejan_Maksimovic Member - All Emails Posts: 225
    via Email
    > The WDK is using standard library files that are older than some people
    > here. What is preventing getting these updated to the latest file set? The
    > language has marched a long way since the 90's. No one is asking for
    > exceptions and i/o. But certainly std::byte, std::array, std::unique_ptr,
    > and others would be terrific to have in the kernel arsenal. The key here is
    > to allow use of standard things (std::) that are well documented and
    > understood by developers, not proprietary wil:: things copy and pasted from
    > std:: concepts. And the foundation of any higher level c++ library should be
    > built on std:: building blocks.


    Hmmm. IMO, anything that allocates/deallocates behind the scenes would
    be a bad bad bad, very bad idea in the kernel.
    It would take a lot of time for some (as old as myself :)) to not
    think about those, but newbies will certainly run at it. And unplanned
    allocations/reallocations are a bad bad bad bad thing here ;) I know -
    I crashed servers becuase of it, when I started (for not using
    lookasides).

    std:array/list/map and even unique_ptr sound exactly like the kind of
    stuff that would be the problem, without the user realizing it,
    without considerable previous experience.

    My 5c on this matter. I love the wrappers around kernel
    primitives/structures though.
  • Peter_Viscarola_(OSR)Peter_Viscarola_(OSR) Administrator Posts: 7,213

    Awesome, Mr. Tippet. Thanks for taking the time to do such a nice write-up.

    I’m looking forward to playing with this library.

    Sounds like a step in the right direction. Some day, we Windows kernel mode devs will actually arrive in the 21st century.

    Specialty libraries like this raise an interesting question, however: Is their use “worth it” in terms of the potential for confusion “down stream” that they very well might cause? It’s pretty simple (sort of) for us all to agree in vanilla C or “a little C++ as a better C” — but are most devs ready for, say, wil::scope_exit? It’s an interesting problem. Different devs, with different levels of experience and coming from different heritages, will all have different views on what’s “good code” (and we don’t have Cutler handy to offer to “put [someone] through a wall” for violating the standards). For example, I find

    RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));

    heinous enough that I’d require it to be changed. But it becomes religion and reasonable folks will differ.

    Thank you again for your many contributions here, not just this latest one.

    Peter

    Peter Viscarola
    OSR
    @OSRDrivers

  • Jeffrey_Tippet_[MSFT]Jeffrey_Tippet_[MSFT] Member - All Emails Posts: 513

    For example, no object for KSPIN_LOCK and thus no guaranteed initialization

    This is on its way. Feel free to file an issue on github or complain here if you'd like to see any other types.

    What is preventing getting these updated to the latest file set?

    Finding the time to do it. We seem to have finite hours in the day, and infinite tasks to work on... just like, I'm sure, everyone else here.

    IMO, anything that allocates/deallocates behind the scenes would be a bad bad bad, very bad idea in the kernel.

    A big part of C++'s offering is that it can hide things. E.g., it can hide a call to Dereference, Unlock, Free, Close, LowerIrql, or CompleteRequest. Whether you want this in your codebase is a matter of taste, and I can't ever see Microsoft ever forcing this decision on a driver writer. If you can't grep for all the Free calls because they're invisible, how can you possibly debug a memory leak? But the counterargument is that if the Free code is automatic, there aren't memory leaks. Whether you & your codebase buys into that is your choice.

    potential for confusion “down stream”

    This is very real, and something we've been (and still are) working through internally. At first, a lot of devs were startled to see bits of wil spackled throughout code reviews, and there were more than a few "WTF is this?" comments. Over time, though, the thing has caught on in many parts of Windows. So just as every win32 developer knows about the GetLastError() dance, we're getting to the point where most win32 developers (internally) know about RETURN_IF_WIN32_BOOL_FALSE. (That macro is the equivalent of the GetLastError() dance, with some logging thrown in.)

    Nobody (at this point, at least) is really pushing wil outside of Microsoft, but there's some hazy dream of a future where every win32 developer just knows these macros and types, and it would look weird to manually spell out the error-checking or handle-closing.

  • Dejan_MaksimovicDejan_Maksimovic Member - All Emails Posts: 225
    via Email
    I think you misunderstood me.
    There are three different issues here:
    - list/map/array doing allocations/deallocations/reallocations behind
    the scenes, quite often. In case of a file system filter, this can
    easily cause memory fragmenttion issues. Without a lot of control of
    granularity of allocations (reallocates on 128 item boundary for
    example, rather than some random/internal number of items..), or a way
    to use lookasides in some way.
    - A class that internally allocates a variable sized pool, and is used
    extremely often. CLogInformation, as a DbgPrint alternate comes to
    mind here. We cannot address something like this, without changing the
    code to use lookasides (what we do in our code whenever strings are
    used)
    - General carelessness for allocations/deallocations. This is not a
    matter of tracking or anything us oldies may need to worry about. But,
    and I am thinking out loud here, rather than with some concrete data
    like above (IANAP - I am not a psychologist :)). I think this will
    cause newcomers to rely heavily on such structures, and often forget
    that the rest (most?) of the code needs manual cleanup.

    So, for the first two, I have concrete suggestions: design the
    structures or add helper APIs that would mitigate memory
    fragmentation.
    For the third one - I have no concrete idea :)


    The reason this is on my mind right now is that it just so happens
    that my current project would feel issues #1/#2 in a few hours :)

    >> IMO, anything that allocates/deallocates behind the scenes would be a bad
    >> bad bad, very bad idea in the kernel.
    >
    > A big part of C++'s offering is that it can hide things. E.g., it can hide
    > a call to Dereference, Unlock, Free, Close, LowerIrql, or CompleteRequest.
    > Whether you want this in your codebase is a matter of taste, and I can't
    > ever see Microsoft ever forcing this decision on a driver writer. If you
    > can't grep for all the Free calls because they're invisible, how can you
    > possibly debug a memory leak? But the counterargument is that if the Free
    > code is automatic, there aren't memory leaks. Whether you & your codebase
    > buys into that is your choice.
  • Tim_RobertsTim_Roberts Member - All Emails Posts: 12,969
    via Email
    On May 11, 2019, at 1:36 AM, Rourke wrote:
    >
    > The WDK is using standard library files that are older than some people here. What is preventing getting these updated to the latest file set? The language has marched a long way since the 90's. No one is asking for exceptions and i/o. But certainly std::byte, std::array, std::unique_ptr, and others would be terrific to have in the kernel arsenal.

    Developing, testing, and maintaining a C++ standard library is an enormous job. ENORMOUS. The special cases and performance guarantees are explicit, onerous, and painful, and there are relatively large changes every 3 years.. I don’t believe any kernel developer is qualified to do that. It has to be someone from the compiler team, and they are busy with more important things.

    Tim Roberts, [email protected]
    Providenza & Boekelheide, Inc.

    Tim Roberts, [email protected]
    Providenza & Boekelheide, Inc.

  • Jeffrey_Tippet_[MSFT]Jeffrey_Tippet_[MSFT] Member - All Emails Posts: 513
    edited May 11

    list/map/array doing allocations/deallocations/reallocations behind the scenes [...] A class that internally allocates a variable sized pool

    Gotcha. Currently, wil doesn't include any data structures (list, array, tree, etc). Currently, wil is more about wrapping existing functionality. Since wil is cobbled together from the contributions of dozens of engineers who were all scratching some itch, it's hard to say that it'll never have this or that feature. But it's more likely to get a wrapper for RTL_GENERIC_TABLE than to get its own tree written from scratch. So wil tends to not directly allocate things; it tends to wrap some C-language API that does the allocation.

    NDIS does have its own generic data structure, which we've published here. But this will probably not graduate into wil. (Unlike, say, the spinlock and Ex-rundown wrappers in the same directory on that github repo -- those are good candidates for wil.)

  • Vadim_SirotnikovVadim_Sirotnikov Member Posts: 16

    On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you're locking for? Every place I've worked in defined "logging" differently.

  • Vadim_SirotnikovVadim_Sirotnikov Member Posts: 16

    I also recall writing something of an std::string / std::wstring wrapper for safe string, and UNICODE_STRING types, though we made some assumptions regarding dispatch level safety and unicode tables :)

  • anton_bassovanton_bassov Member Posts: 4,984

    For example, I find RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));

    heinous enough that I’d require it to be changed. But it becomes religion and reasonable folks will differ.

    Well, probably some folks , indeed, just love to do things the way they are done in the sample below

    https://github.com/microsoft/Terminal/blob/master/src/server/Entrypoints.cpp

    If you ask my personal opinion, I would rather do everything in a "plain old C", with all the cleanup done in the end of the function in the reverse order of creation/allocation/initialisation/etc, and a "goto xyz;" jump to the corresponding label in the "cleanup section" in the end of the function upon failure. Call me a retrograde if you wish, but it seems (at least to me) so much easier to read and understand the code written this way.

    Certainly, it maybe not always be possible to do things in so straightforward way, so that in some cases some "ugly" code may have to be written anyway. For example, consider the case when you may have to re-acquire the lock that you have already acquired and released in the same function In this case you would have to release the lock in the "main" code before jumping to the "cleanup section" in the end of the function. Certainly, one may say that the code that does things like that is broken anyway and has to be re-factored, with re-acquisition block being placed in a separate function, but this is already more of a "philosophical issue" that has to be judged on case-by-case basis.

    Anton Bassov

  • Peter_Viscarola_(OSR)Peter_Viscarola_(OSR) Administrator Posts: 7,213

    I think the RETURN_IF_NTSTATUS_FAIL creates too much noise and emphasis on the error path. I’d rather see the main function not buried in an argument, and a go to.

    It’s also too easy for these RETURN_IF_xxx things to grow unanticipated appendages, in special cases, where they return if !NT_SUCCESS(status) ... OR if the status == STATUS_MY_SPECIAL_THING.

    Religion... whatever suits you and those for whom you work,

    Peter

    Peter Viscarola
    OSR
    @OSRDrivers

  • RourkeRourke Member Posts: 31

    @Dejan_Maksimovic said:
    std:array/list/map and even unique_ptr sound exactly like the kind of
    stuff that would be the problem, without the user realizing it,

    std::array does NOT allocate anything behind the scenes, ever. Neither does std::unique_ptr. Neither does anything in the original post about the wil library. No one is talking about lists and maps and behind the scenes allocations in the kernel except you babbling to yourself in error. So maybe before you post you can try to actually understand something about the new library or c++ and then try to make an intelligent comment. And by the way since you've been under a rock note that std::array is now approaching 10 years old and runs circles around c arrays and is a perfect fit for kernel usage. That's why they made it.

  • RourkeRourke Member Posts: 31

    The wil:: library has improved techniques for easy and fail safe freeing of resources. This is one of the big headaches of writing drivers is all the messy and error prone cleanup code. I like these new constructs much better than spaghetti of goto statements and I think if one posted a side by side comparison of the rat's nest ndis code it would grow on people. Kind of like comparing the look of the mass of hand wiring on the back of very ancient mainframes compared to the neat and tidy look of land patterns on a brand new motherboard. And if you balk at all these auto free constructs then I better never see an automatic variable in your c code, anywhere, any kind. Because guess what? Automatic variables are freed automatically behind the scenes for you when they go out of scope--oh the horror!

  • Jeffrey_Tippet_[MSFT]Jeffrey_Tippet_[MSFT] Member - All Emails Posts: 513

    On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you're locking for? Every place I've worked in defined "logging" differently.

    wil's logging is configurable. The full story is documented here. The TL;DR is that you set some #defines to control whether errors get printed to DbgPrint, TraceLogging, or nowhere. You'd presumably have the noisy mode enabled for debug builds, and lower the verbosity for retail.

    I would have liked to see WPP in there -- I'm a fan of WPP. But WPP is not particuarly friendly to being included in someone else's macro; it wants to own the top-level macro. So I'm doubtful that it's technically possible to squeeze WPP into the wil error-handling framework. Well, at least without changing it so much it becomes some sort of WPP 2.0... ;)

    Keep in mind, though, that we haven't done the work to hook up wil's error-handling macros for kernel code yet, so this discussion is only immediately relevent for us in the context of UMDF. The big technical challenge for us is that, while wil's error-handling accepts a variety of types of error codes, it internally converts everything to HRESULT, and its guts use HRESULT exclusively. Obviously that's a non-starter for kernel code; we would need to find a way to plumb NDIS_STATUS, er, I mean NTSTATUS through there. Anything's posisble with enough #defines, but the implementation of wil's error macros is already teetering on the brink of over-abstracted unreadability; adding another dimension of abstraction might cause the moon to fall out of the sky.

    since you've been under a rock

    Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers. We discuss the merits of ideas, not people. In this case, Dejan has expressed a very valid point, and considering that wil has basically no public documentation right now, it's perfectly reasonable to ask if wil runs afoul of these concerns about invisible memory allocation.

  • anton_bassovanton_bassov Member Posts: 4,984

    This is one of the big headaches of writing drivers is all the messy and error prone cleanup code

    Well, you don't necessarily have to use "the tricks of cleanup masters" from MSFT SDK/WDK samples, do you...

    ....these new constructs much better than spaghetti of goto statements

    Actually, "goto" statements don't necessarily have to result in spaghetti code as long as you use them properly (if you need some practical examples you can check Linux and Illumos sources ). OTOH, if you decide to make your code jump all over the place like Mr.Ballmer in a "Monkey Boy " video shown on another thread, "goto" may, indeed, be quite useful for the purpose....

    The wil:: library has improved techniques for easy and fail safe freeing of resources.

    I don't know about the other people, but I wold rather prefer to see the cleanup code in the same function where the target resources have been allocated. This is not just the question of writing the code, but mainly of maintenance. Certainly, this is just the question of taste.

    Anton Bassov

  • Dejan_MaksimovicDejan_Maksimovic Member - All Emails Posts: 225
    via Email
    >> std:array/list/map and even unique_ptr sound exactly like the kind of
    >>
    >> stuff that would be the problem, without the user realizing it,
    >
    > std::array does NOT allocate anything behind the scenes, ever. Neither does
    > std::unique_ptr. Neither does anything in the original post about the wil
    > library. No one is talking about lists and maps and behind the scenes
    > allocations in the kernel except you babbling to yourself in error. So maybe
    > before you post you can try to actually understand something about the new
    > library or c++ and then try to make an intelligent comment. And by the way
    > since you've been under a rock note that std::array is now approaching 10
    > years old and runs circles around c arrays and is a perfect fit for kernel
    > usage. That's why they made it.

    Easy... I did not say they made it, I said if it were to be made, it
    should take a lot of caution. And array is just one of the 3 I
    mentioned.
  • Peter_Viscarola_(OSR)Peter_Viscarola_(OSR) Administrator Posts: 7,213

    Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers

    Too right.

    Mr. Rourke... I’m now officially cautioning you. I don’t like your attitude, in general. And now, you have embarrassed me by your behavior in my house, to the point where another one of my valued guests has had to remind you of your manners.

    Come correct, now. Lose the attitude, and decrease the stridency of your replies, immediately. Or go play elsewhere. Please. Thank you.

    Peter

    Peter Viscarola
    OSR
    @OSRDrivers

  • Vadim_SirotnikovVadim_Sirotnikov Member Posts: 16

    @Jeffrey_Tippet_[MSFT] said:

    On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you're locking for? Every place I've worked in defined "logging" differently.

    wil's logging is configurable. The full story is documented here. The TL;DR is that you set some #defines to control whether errors get printed to DbgPrint, TraceLogging, or nowhere. You'd presumably have the noisy mode enabled for debug builds, and lower the verbosity for retail.

    I would have liked to see WPP in there -- I'm a fan of WPP. But WPP is not particuarly friendly to being included in someone else's macro; it wants to own the top-level macro. So I'm doubtful that it's technically possible to squeeze WPP into the wil error-handling framework. Well, at least without changing it so much it becomes some sort of WPP 2.0... ;)

    Oh wow, I've only dived in to the code some hours after replying, and only then saw the scope. Indeed, it could take more than a single weekend to add WPP support :wink:

    I imagined WPP would be the standard for kernel due to performance and security (erm, obfuscation) considerations, so it could be on by default when deployed. If I'm missing some obfuscation ability in TraceLogging I'll be delighted to get a push in the right direction.

    In any case, this is great work. I'm excited to see modern C++ idioms in Kernel space.

  • Michal_VodickaMichal_Vodicka Member - All Emails Posts: 50
    edited May 13

    Nobody (at this point, at least) is really pushing wil outside of Microsoft, but there's some hazy dream of a future where every win32 developer just knows these macros and types, and it would look weird to manually spell out the error-checking or handle-closing.

    For me it looks like a nightmare. Marcos which influence code flow (contain return or goto) are IMO plain evil. They make code shorter but less readable and in turn less robust/reliable. Call me dinosaur but I prefer when the code is readable as a book and code flow is explicitly expressed. Here you're trying to express it in the macro name which is an interesting idea but it leads to longer macro names and makes this problem worse:

    I think the RETURN_IF_NTSTATUS_FAIL creates too much noise and emphasis on the error path. I’d rather see the main function not buried in an argument, and a go to.

    I've seen too much code plagued by REQUIRE-like macros and it is far less readable than longer version which handles errors explicitly. For this reason. You see just a chain of REQUIREs and what is code really doing is hard to decipher.

  • mmmm Member - All Emails Posts: 1,410
    via Email
    You're one of my favorite dinosaurs :)


    mm
  • Michal_VodickaMichal_Vodicka Member - All Emails Posts: 50

    Creating fossils museum? ;-)

  • RourkeRourke Member Posts: 31

    Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers.

    Totally agree and I would never do that. A bigger problem I see is people who carelessly post false or misleading information in a forum dedicated to professionals helping each other. Faulty content has no place here and should be called into question for the benefit of the poster than made it and even more for others that read it. There is nothing personal about setting the record straight.

  • RourkeRourke Member Posts: 31

    RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));

    My first reaction when i saw this is it's extremely ugly and breaks a lot of rules and no way I would ever want to see that in code. But thinking about it some more this is really just a poor man's exception. Almost a throw statement if you will. If you look at it in that light it then becomes a thinking point on how it might be useful.

  • anton_bassovanton_bassov Member Posts: 4,984

    But thinking about it some more this is really just a poor man's exception. Almost a throw statement if you will.

    Well, a throw() statement is just a language construct that one may use or misuse the way they wish. Let's say we have a function that may throw an exception, and this function may allocate resources and/or lock semaphores before doing so. As I have said already, I would personally prefer to make all the cleanup in the same function in case of a failure. The use of throw() statement does not preclude me from doing so in any possible way, does it - my callee can make its "internal" cleanup before throwing the exception, and let the caller do its "external" one in a catch() block.

    However, RETURN_IF_NTSTATUS_FAILED() does not seem to offer me any possibilities to structure my code this way, does it.
    Therefore, it does not really look like a throw() statement, at least from my perspective. OTOH, someone who would prefer to handle all the cleanup in a caller's catch() block may, indeed, think of it as just of some kind of a throw() statement.

    In other words, this is, again, just the question of personal taste and preferences....

    .

    Anton Bassov

  • Peter_Viscarola_(OSR)Peter_Viscarola_(OSR) Administrator Posts: 7,213

    Totally agree and I would never do that.

    Bullshit. Just so we’re clear, regardless of what you think, if I think you do it one more time, I will put you on moderation.

    Are we clear, Mr. Rourke?

    Peter

    Peter Viscarola
    OSR
    @OSRDrivers

Sign In or Register to comment.

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Upcoming OSR Seminars
Developing Minifilters 29 July 2019 OSR Seminar Space
Writing WDF Drivers 23 Sept 2019 OSR Seminar Space
Kernel Debugging 21 Oct 2019 OSR Seminar Space
Internals & Software Drivers 18 Nov 2019 Dulles, VA