Annotating function parameters and using PREfast to check them


Via the suggestion box, Sys64738 asks, whether I think is a good C/C++ programming style to add IN and OUT to function prototypes.

Remember, this is my opinion. Your opinion may validly differ.

I would go beyond just IN and OUT and step up to SAL annotations, which describe the semantics of function parameters in more detail than simply IN and OUT. For example, the __out_ecount annotation lets you specify not only that the buffer is an output parameter, but also lets you specify how big the buffer is.

This added expressiveness is used by tools like PREfast and its user-mode equivalent the C/C++ Code Analysis tool. These tools study your source code and attempt to find errors like writing past the end of an output buffer, passing the wrong buffer size for an output parameter, or writing to an input buffer.

One question that often comes up when people start adding SAL annotations to their code is "I have a function whose parameters are used in a manner that isn't neatly covered by an existing annotation. How do I annotate it?" If you ask them to describe the parameters they are having trouble with, you often find that they don't have an easy annotation because they are too complicated. "Well, this is a null-terminated input string buffer, unless the SET flag set, in which case it's an output buffer with length specified by the cchBuf parameter, and the buffer size is specified in CHARs unless the FN_UNICODE flag is set in the dwFlags parameter, in which case the size is in WCHARs."

One lesson we learned in Windows is that if your function is so complicated that the annotation language has trouble expressing it, then that might be a clue that your function is so complicated that human beings can't understand it either. The answer to "How do I annotate it?" is therefore "Don't try. Simplify your interface instead." (Of course, if the function must be kept around for compatibility reasons, then you are stuck with a complicated annotation.)

Comments (20)
  1. I'll go a step further and say "make your __in arguments const".

  2. Joshua says:

    @Maurits: I wouldn't, unless they're pointers. I like to fiddle with my incoming arguments.

  3. Marquess says:

    @Joshua:

    What a coincidence, I like to argue with my incoming fiddlers!

  4. Alexandre Grigoriev says:

    'const' qualifier is a good thing, but as recently as in WDK 6000, MS C compiler used to have problems with it. I suppose when it compiles C++ files, it should be OK, because 'const is used more often there, but with C files the compiler was often confused.

  5. CodeOrDie says:

    Uh oh, this is about to devolve in one of those thread where everyone leaves out one detail of what they understand to be the problem and it becomes a huge argue-fest even though we're all very smart. So… I'll add my piece of coal to this BBQ!

    1. foo(__in int x)
    2. foo(__in const int& x)

    3. foo(__in const int x)

    In all cases, the caller is guaranteed the value of the passed parameter post-call, it will be whatever it was pre-call. But, inside the function, you can "fiddle" with x in case 1., but not in case 2. Case 3 is pointless, I think.

    Of course, we know that case 2. is a performance optimization. But is this integer copy really a big bottleneck in your app? And, though Joshua said he "likes to fiddle" with arguments, I think he really meant: "I like to keep open my opportunities to not wonder whether it is safe to transform this innocuous parameter".

    (Sorry for the double-post if there is one.)

  6. Timothy Byrd says:

    @Marquess:

    What about the Cellists?

  7. Jeff says:

    Uhh…  tangential question for you:  what does the PRE in PREFast stand for?

    (apologies if it's obvious, I didn't drink my coffee today)

  8. dasuxullebt says:

    With that much of annotations, one could as well do proper formal program verification.

  9. FWIW, I was thinking of pointers when I made my suggestion.  I don't see much… er, value… in annotating non-pointer arguments.

  10. William says:

    @Joshua:  If you've declared them as __in in the header, you've explicitly stated that you don't write to that buffer.  In other words, you have specifically told the caller that you *won't* "fiddle" with them, just read them.

    If you really need to transform your inputs, you need to either make a copy or declare the parameter as __inout.

    [I think you're talking about different things. Joshua is talking about foo(__in int x) vs foo(__in const int x). -Raymond]
  11. Medinoc says:

    @Maurits: Better yet, make your const arguments __in, not __inout (I'm looking at you DrawText()).

  12. Hmmm… looks like DrawText supports a DT_MODIFYSTRING flag which can modify lpchText.  So __inout is correct and the "LPCTSTR" is a bug (const implies __in, so it's not necessary to add __in to pointer-to-const arguments.)

  13. stewart says:

    @Maurits: but in the case of DrawText you could argue it is the better of two evils. Either MS change the argument type to LPTSTR and the 99.9% of people who do not use DT_MODIFYSTRING have to insert a const_cast, or more likely simply don't use const for their strings to avoid the error you get, or they cheat and call the argument LPCTSTR, do the const_cast internally where appropriate and the 0.1% of people who use DT_MODIFYSTRING have to remember that the function prototype is wrong.

  14. Medinoc says:

    @Maurits: But that's the problem: DT_MODIFYSTRING, and the inability to annotate that it's not always there, are exactly what Raymond advised against: It should have been in a separate function.

    Also, a function that can write to a const pointer is an ugly kludge.

  15. SDL Team says:

    const params are treated as __in by default.

  16. Jonathan says:

    Jeff: I've always thought it's from PREfix. PREfix is another static code analysis tool used by Microsoft*. PREfix runs on dedicated machines, and typically takes days to complete a run. On the other hand, PREfast runs on the developer's machine fairly quickly. So in my mind, it's "PREfast = PREfix, but faster!".

    PREfix itself was an acquisition (company's name was Intrinsa, I think), so I don't know where this name comes from.

    Corollary to Raymond's last paragraph: If you can't find a good name for a function, then you don't know what it does, and you need to refactor. Same for variable names.

    Annoying mention: Obviously, the above was understood only after FormatMessage() was created…

    * Mentioned here: technet.microsoft.com/…/cc723542.aspx

  17. cashto says:
    1. C and C++ are different languages. There is no such thing as "C/C++".

    2. In C++, if you are dealing with raw buffers whose size is held in a separate variable, you are Doing It Wrong.

  18. arnshea says:

    What an excellent way to reinforce a concept that strikes me as a close cousin of "If it's too hard to name then it's probably doing too much".

  19. Work harder says:

    @arnshea

    Or too less.

    DoNothing();

    DoNothing2();

    DoNothingAlso();

    DoNothingAgainAndAgain();

  20. There is no such thing as "C/C++".

    On my team we call it "C+".

Comments are closed.

Skip to main content