Why does WDF have its own (Wdf)False?

I mean, come on! Another false value? You have the C++ false, the Windows FALSE and now another one. Furthermore, another boolean type (BOOL, BOOLEAN, bool)? … at least we didn't add one a traditional Boolean type. What gives? If I were not a member of the WDF team I would question their design skills, but since I am a member of the team that designed the usage of this value, I feel I have some explaining to do. WdfFalse is a part of an enumeration, WDF_TRI_STATE, which also defines WdfTrue and WdfUseDefault.

typedef enum _WDF_TRI_STATE {
WdfFalse = FALSE,
WdfTrue = TRUE,
WdfUseDefault = 2,

As you can see, WdfFalse is equal to FALSE (as WdfTrue is equal to TRUE). Why would we redefine the traditional TRUE and FALSE values instead of just using the traditional values as is? Well, WDF has data structures (WDF_DEVICE_PNP_CAPABILITIES, WDF_DEVICE_POWER_CAPABILITIES, etc.) with lots of BOOLEAN like fields in them and we wanted to be able to only set a subset of them at a time and then call a DDI. What we didn't want is that the developer had to set each field before making the call. The framewoek had to be able to tell if a field was intentionally set or was just the default value as set from the INIT() routine (so that it could be ignored). There are 2 common ways implement this pattern. The first way is to have a separate field in the structure which is a bit field of flags and you set a flag in this field to indicate that another field was set (many Win32 common controls use this pattern, especiall the listview and treeview controls). The second way is to choose a value which indicates that the field should be ignored. For a BOOLEAN value, only TRUE and FALSE are valid values, but we didn't want to define a value like NOT_TRUE_OR_FALSE, so we created a new enum to represent true, false, and not set. WDF_TRI_STATE is that enumeration.  Originally WDF_TRI_STATE looked like this:

typedef enum _WDF_TRI_STATE {
WdfUseDefault = 0,
WdfTrue = 1,
WdfFalse = 2,

This made the the INIT() routine very simple since WdfUseDefault was zero. The INIT() routine could zero the structure (which would set all of the WDF_TRI_STATE fields to WdfUseDefault), set the Size field and then return (assuming all of the fields were WDF_TRI_STATE, if not, other initialization might occur). Conceptually this enum definition was clean and simple. The problem was that developers treated the WDF_TRI_STATE fields as BOOLEANs and used TRUE and FALSE instead of WdfTrue and WdfFalse. This meant that if the developer assigned FALSE to a field, it was not really false, it was WdfUseDefault (which meant the field was ignored)! To make matters worse, the compiler never flagged the error because TRUE and FALSE are #define constants, not real enumerants and C/C++ is very loose about translations between constant values and enumerations. We created a type that was almost always used incorrectly and the error was not easily discoverable. Very bad for usability and writing correct the first time.

By the time we realized this problem, getting rid of WDF_TRI_STATE was not viable (and besides, getting rid of it didn't do us any good, we still needed a way to set a few fields and know how to ignore other fields), so we changed the definition of WDF_TRI_STATE to its current state. The way it is currently defined makes it easier on the driver writer, WdfFalse and WdfTrue map to the traditional FALSE and TRUE which means they can be used interchangably.  More importantly, there is no undiscoverable mistake for the driver writer to make. On the other hand, this definition does place the burden on WDF to manually initialize each WDF_TRI_STATE field the structure correctly in the INIT() routine. For any structure which has a WDF_TRI_STATE field, the structure must be zeroed, the Size field is set, and then each WDF_TRI_STATE field must be set to WdfUseDefault (see WDF_DEVICE_PNP_CAPABILITIES_INIT in wdfdevice.h for an example of all the gory details).

So, in the end, we were not crazy or out of our minds :). We were well intentioned, got bit by a usability bug and a lack of type safety in the compiler and altered our design accordingly. In retrospect, we could have used values like WdfYes and WdfNo instead of WdfTrue and WdfFalse, but these alternate names would still be equal to their BOOLEAN counterparts and I don't think it woudl have been any better in the end.

Comments (3)
  1. Igor Levicki says:

    Wasn’t it much easier to ask the users to set the highest bit of each value they want to set?

    #define WDF_SET 0x80000000

    struct ss;

    ss.WantThis = TRUE | WDF_SET;

    That way you just have to zero the structure, and you could ignore every field whose sign bit is zero, not to mention that you wouldn’t need an enum nor an additional field for bitmask.

  2. setting a bit would work as well, but that is a pretty large departure from what a developer would expect to do.  We called these types of design decisions "cliffs," as in how much of a conception cliff do we want to create and far is it for the developer to fall off of ;).  

    In this case, I do not see that the bit adds any more value then our own enum.  We do not have a bitmask field to indicate which fields are set (unlike some common control structures) and the enum is interchangable with TRUE/FALSE.  The onus of setting WdfUseDefault is on the framework developer, not the WDF client driver writer so it is much easier to isolate the cliff internally.

    thanks for the comment,


  3. Igor Levicki says:

    But it does add more value since it could be used with numeric fields or larger enums/bitfields as well, instead of only with Booleans.

Comments are closed.

Skip to main content