Share via

PFD Annotations - They're just comments, really.

  • Article

As someone that has done development work, I know the joy experiecned when asked to better comment my own code after I’ve completed it. It works, I know what it does, and I would rather work on a different feature than go back and change something I’ve already completed. It doesn’t feel like progress; it feels like regress until I can clear all of that other work out. I have these feelings knowing full well that comments are incredibly beneficial, and knowing that for as much as I have thought my own code to be clear and concise without the comments, revisiting the code even just a few months later requires time to become reacquainted with it when those comments aren’t there. The case for commenting has been drilled into every aspiring Dev’s head somewhere along the way, so restating it beyond what I’ve already said isn’t necessary.

Both PFD and SDV make use of SAL (standard source code annotation language) annotations in the code to assist with their checking engines. Both will use these to determine the correct usage of a function, and to warn the developer against things that will cause potential troubleshooting pain later on. SAL annotations have been in place in some form for while – the easily recognizable “IN”, “OUT”, and “OPTIONAL” annotations are found in abundance in the current Windows 2008 WDK. When code compiles, these resolve out to nothing, they’re simply in-code comments that help the developer get a quick idea of what a particular parameter is used for. The annotations for PFD and SDV take this to another level in both code readability and usability of these tools.

Take an example of a function, defined in wdm.h in the Windows 2008 WDK, that seems reasonably easy to determine what it does, based on it’s name:

NTKERNELAPI

NTSTATUS

NTAPI

KeSaveFloatingPointState (

    OUT PKFLOATING_SAVE FloatSave

    );

This function is to be used whenever floating point arithmetic of any sort is to happen in Kernel code. Calling this before doing any sort of operation relieves you of the possibility of nasty little issues that can occur with the floating point registers if you’re switched out while executing your codepath. Based on what is seen above, we know it’s a Kernel API, it returns NTSTATUS and a structre we will use when we need to call KeRestoreFloatingPointState to restore it. Neither wdm.h or fltsafe.h (where the FLOATSAFE struct is defined) provides any other real hint as to concerns we should have for this function call. A trip to the MSDN page for KeSaveFloatingPointState is necessary to tell me that it’s okay to call this at <=DISPATCH_LEVEL.

Consider the information provided when appropriate PFD annotations are added:

__checkReturn

__drv_maxIRQL(DISPATCH_LEVEL)

__drv_valueIs(<0;==0)

__drv_when(return==0, __drv_floatSaved)

__forceinline

NTSTATUS

KeSaveFloatingPointState (

    __out __deref __drv_neverHold(FloatState)

    __drv_when(return==0, __deref __drv_acquiresResource(FloatState))

    PVOID FloatingState

    )

Based on this information, I now know, without a trip to my browser and MSDN:

- It is not safe to assume this function is always successful. While the fact that it returns a value of any sort should cement this belief anyway, the annotation “__checkReturn” acts as the concrete.

- The maximum IRQL level that this function can be called at is DISPATCH_LEVEL, so it’s safe to call from a DPC, but not my ISR; and should relieve me of the worry of paged code when calling it.

- The value returned is either less than 0 or 0.

- When the value returned is zero, the floating point state is saved.

The parameters themselves also hold annotations, which admittedly take a bit more getting used to than looking past simple “IN” or “OUT” annotations. The annotations applied to FloatingState translate out to:

- When this function returns STATUS_SUCCESS, it acquires the resource that we’ll call ‘FloatState’. With that in mind, the ‘FloatState’ resource should never be held when calling this function.

- The __deref is part of properly pointing to the resource that the FloatingState pointer is pointing to.

- The __out annotation is self-explainatory: we’re expecting this function to provide us with something useful to point to with that parameter.

For informational purposes only, those annotations realized the same effect as a simple comment placed before the function defination would have. Review of the SAL annotations leaves a developer with a far better idea of what an API is capable of, regardless of any documentation they may have found on the subject. For example, the annotation makes it apparent that holding the FloatState and attempting to save it again is not a good idea; the MSDN write-up doesn’t mention this.

Annotations can range from very complex and situational down to “duh” for any programmer. Checking the return value of a function is generally a good idea, but you can not do so and nothing in the compiler will stop you. You can also try to drink coffee in large gulps the instant the Barista hands it to you, too, but that doesn’t mean it’s a good idea. It’s why coffee cups have “CAUTION: HOT” written on them these days, and why an annotation like “__checkReturn” exists. They’re safety measures, and help you keep yourself from getting hurt if you follow them, no matter how trivial they may be. The __checkReturn annotation creates a warning for anyone that decides to ignore the status, and much like “CAUTION:HOT”, they can continue to do so possibly at their own peril - I’ve seen a number of crashes simply because a status was ignored.

Whether they’re to be considered fancy comments or warning signs, the benefits of these annotations especially when paired with PFD result in APIs used in their intended manner and less crashes on the user desktop. Problems that can create crashes are brought to the developer’s attention during development, not when users of the beta are clamoring for a fix – or even worse, after release. Application of these annotations for a developer familiar with their DDI is minimal, the savings potentially incredible. As a developer, if applying these means finding a bug in my code before releasing it, the time invested in application has paid for itself already – for basically nothing more than providing information that I should be putting in comments anyway.

Comments