Annotations for NULL Dereference in Prefast and Compiling on GCC

Question

I have the following example code and would like to know what kind of annotations I can use to avoid them.

int Function(classA* pInput) {
   if (pInput == NULL) {
      classA::Create(pInput);
   }

   return pInput->value;
}

The problem is that since Prefast evaluates only the function it doesn't know that Create initializes the pointer.

I thought I could solve it by using the __out annotation in the header file for classA::Create however that didn't work.

I'm wondering if there is a good alternative to just __analysis_assume everywhere in the code such that prefast picks it up from the function definition.

Secondly, I was wondering how would I set up my build configuration so that I can build my code natively on Linux or with GCC with these preprocessor directives. Would I have to check if it's on a LINUX build and then add those annotations as empty Macros?

I've seen this indentation style a _lot_ over the past week or so. Is it from some textbook? — Lightness Races in Orbit, Sep 20 '11 at 17:46
There is nothing wrong with that code assuming `Create` is a static method that actually allocates and initialize the object. Are you getting an error? — AJG85, Sep 20 '11 at 17:46
I'd like to forward a motion to burn this indentation style with fire. — Collin Dauphinee, Sep 20 '11 at 17:48
Seconded, although I don't Tom's edit either. I'm one of those horrible ones who puts brackets on their own line with proper spacing of code so it's easy to see each scope block. — AJG85, Sep 20 '11 at 17:50
It triggers the following warning: 6011 (http://msdn.microsoft.com/en-us/library/windows/apps/ff539809(v=vs.85).aspx) — Setheron, Sep 20 '11 at 17:55
The reason being is that prefast has no way to know that Create is guaranteeing to initialize the pointer? — Setheron, Sep 20 '11 at 17:56

score 2 · Accepted Answer · edited May 23 '17 at 11:55

MSalter’s answer sounds very much like the correct technical answer. Happily I don't know SAL so I can't say for sure, but it looks like the technical level solution.

However, I would just recommend that you rewrite your current code …

int Function(classA* pInput) {
   if (pInput == NULL) {
      classA::Create(pInput);
   }

   return pInput->value;
}

as …

int Function( classA* pInput ) {
   if (pInput == NULL) {
      pInput = classA::Create();
   }

   return pInput->value;
}

The main issue here is whether you're leaking a dynamically created object, or not. That depends very much on what the Create method does. But it does look like you're leaking.

And in that case, where Create does nothing but dynamically allocate a default-initialized classA object, then here is how you can do that more safely and more efficiently:

int Function( classA* pInput ) {
   if (pInput == NULL) {
      return classA().value;
   }

   return pInput->value;
}

Finally, for a total clean-up consider how to get rid of unnecessary raw pointers. Because raw pointers create problems (your code being just a very small example of that). Then you can do things like this:

int function( ClassA const& anObject = ClassA() )
{
   return anObject.value;
}

Essentially this is a C++ level solution, as opposed to the original code's C level. So I changed also the naming convention to reflect the much stronger focus on types at the C++ level. Here the types have uppercase first letter, and a mere function has lowercase first letter.

It's simpler, it's safer, it's again more efficient.

And – at the C++ level you generally don't have to struggle with silly SAL notations. :-)

Cheers & hth.,

+1 for the leak detection, +1 for suggesting to use a reference rather than a pointer, -1 for not suggesting to throw an exception in the case of a null pointer. Net: +1. — David Hammen, Sep 20 '11 at 23:33
@David: I chose to treat the code as intentional, and in that case a nullpointer is not an error but the designed way to get a default. Now gimme the extra upvote! — Cheers and hth. - Alf, Sep 21 '11 at 00:09
I tried but I got this message: "*You last voted on this answer 1 hour ago. Your vote is now locked in unless this answer is edited.*" — David Hammen, Sep 21 '11 at 00:51

MSalters · Answer 2 · 2011-09-20T19:46:51.677

1

Seems like SA is missing [Post( Null=No )] on the classA*& parameter of classA::Create(classA*)

edited Sep 20 '11 at 19:46

answered Sep 20 '11 at 19:22

MSalters

173,980
10
155
350

I cannot simply add __out to ::Create? – Setheron Sep 20 '11 at 19:43
I don't think so. `__out` merely guarantees that `pInput` has _a_ value, not that it has a non-null value. Note that `[]` is [the new syntax](http://blogs.msdn.com/b/michael_howard/archive/2006/05/19/602077.aspx) – MSalters Sep 20 '11 at 19:54

Annotations for NULL Dereference in Prefast and Compiling on GCC

2 Answers2