Improving software quality using Visual Studio 11 C++ Code Analysis

Excellent session. Really simple and small and concise code demos. Worth going to it. Looking forward to using SAL.

Having tried CA in the developer preview, here are my comments:

1. It's still overrun with warnings from the tool supplied headers, both SDK,CRT, & ATL.

2. It still only appears to work on a whole project - I can't see any facility to do CA on single source files, which would be very useful!

3. This one appears to be new (I don't recall seeing it before):

C6054: String 'VarName' might not be zero-terminated.
What's the analyser looking for that's giving rise to the above?
i.e. how do you code to prevent it?

The situation it's arising in for me is a fairly normal use of _splitpath():

TCHAR szDrive[_MAX_DRIVE];
TCHAR szDir[_MAX_DIR];
TCHAR szFName[_MAX_FNAME];
TCHAR szExt[_MAX_EXT];

_tsplitpath_s( _T("C:\\Path\\Filename.exe"), szDrive, szDir, szFName, szExt );
int ExtLen = lstrlen( szExt ); // << C6054 here

4. In the IDE Code Analysis pane, the hover over highlighting is really annoying as if you happen to leave your mouse pointer over the list and then use the keyboard for navigation; it looks like you have multiple items highlighted.

Can you either:
i) Eliminate the hover highlight - preferable, the Vista introduced mouse hover highlighting of items in lists has always irritated me.
ii). Change the hover colour to be noticeably different to the selected item colour.

5. Using the IDE facility to supress a CA warning in code - inserts a pragma suppress line.

However, this only works for the first such edit on a file and each subsequent insertion is off by the number of lines previously inserted. i.e. there's a fundamental issue here of needing to track the original line of code!

6. You know this one is missing! 64-bit compiler support.

David, thank you for your feedback. Enabling code analysis support for 64-bit is among our top priorities.

Thank you for very interesting session.

In your presentation you shown the 'void Print(_In_ int * p)' function that demonstrates the _In_ annotation, meaning "pointer may not be null, pointed value has to be initialized".

In your opinion, does it have sense if further development of Code Analyser leads to certain language features, so that programmers are able to define three Print functions instead of one, each optimised for specific case? The first is for the case when analyser detects that the pointer is not null, therefore the function can dereference it without concern. The second is when the pointer is definitely null; this version of Print will report a compile-time error. And the third is when analyser cannot determine the previous cases; this version will contain an 'if' that detects wrong null values and throws run-time exception.

@David - Thank you for your feedback. 1. We are working to reduce the number of warnings in the headers as much as possible.  2. Running code analysis on one source file is an interesting idea, we've taken note. 3. Nice find! This is due to a missing annotation on _tsplitpath_s() that tells the analyzer it indeed returns null terminated strings.  We will investigate.  4 & 5. Great suggestions.  The second one is something we have discussed as well and we are looking into possible solutions. 6. As Jason mentioned, 64bit support is a high priority for us.

@Viorel - Thank you for your interest in code analysis.  Code analysis is a static analyzer, meaning it solely depends on the source code, and does not require any changes to the binary to work correctly.  There are big advantages to this methodology (no performance or size impact, minimal testing infrastructure required, consistent results) that would have to be compromised in order to throw runtime exceptions.  I'd be happy to discuss further if you have additional questions.

Vince

I really liked the new features of CA in VC11, specially the new annotation _Guarded_by_

The new-view, detailed explanation and filtering feature is also excellent improvement!

And the greatest thing MS is doing - giving CA to everyone for the software eco-system. Kudos!

Now, few of my concerns, which I can think of now:

• Functions like EnterCriticalSection, WriteFile, do depend on some handle. There should be feature to detect if relevant "open/create" is called successfully or not. Some annotation may be given to link these "open" and "use" functions, which would also facilitate developers who develop modules.
• Checked headers, but couldn't get how ECS and LCS are related, and CA is issuing Race condition warning against them. But there should be another annotation, for other developers, to link them - so that two functions like Begin--End may be analyzed.
• There is no feature to detect dead-code - for global functions and for unused members of class. This is, I believe, much easier. The linker optimization would (may) generally remove unreferenced code. It would be great if CA also does to same to let developer know.
• It goes for unused classes, enums etc. also.
• If a class method is not modifying the data or calling other non-const method, but it is currently a non-const method - then CA should mention it may be marked as 'const'
• Most compilers are having "uninitialized variable" warning, but no static-analysis tool seem to have using uninitialized member-variable - if not initialized in constructor. If constructor (or some initializer called from constructor(s)) doesn't initialize some variable, CA warning should be popped if other method tried to "read" from member variable.
• Code Analysis detects most of memory-function bugs, but it allows non-POD objects to be initialized/copied using these functions. Let's have CString object(s) in some class, and initialize this class object using memset/memcpy! You got me, right?!
• Contract between caller and callee is made - but contract between prototype and the implementation is not made!! Consider following code:
  

  void Copy(_In_ char*);
  
  void Copy(_In_opt_ char* p)
  {
  *p=0;
  }

  Here, compiler sees first prototype, and records it. There is a warning of nullptr is passed, but no warning in function implementation, where pointer is used without null check!