Question

I've inherited a large C++ codebase for several Windows applications that successfully is in use by many customers.

  • The codebase is large, >1mill LOC.
  • The codebase has a history of 15+ years.
  • The codebase is in some areas dominated by C programming style and/or not very modern C++ style, e.g. not using Standard C++ collections and algorithms.
  • The codebase has unfortunately only been compiled with warning level 2 (/W2 in Visual C++). I would like to increase to level 3 (/W3) to increase security and prepare for 64-bit.

The most problematic point in the increase to warning level 3 are the many warnings received involving signed/unsigned mismatches and I recognize that it will be a very large task to resolve all those for the existing codebase.

What would be a good approach to ensure and enforce that new code committed to the codebase are compiled with the increased warning level?

In more general terms the question could be rephrased to how you enforce increased programming quality into new committed code. If you don't do anything, new code has, in my experience, a tendency to be affected and styled similar to existing code, rather than being improved to more modern standards.

Was it helpful?

Solution

I would even go as far as going to warning level 4 (/W4).


Since you're using Visual Studio, it's quite easy to suppress bothersome warnings like signed vs unsigned comparision:

#pragma warning(disable:NNNN)

Where NNNN is the number of your warning. Now put all those disabled warnings in a header file (say, "tedious_warnings.h") and force-include that header file everywhere - Project Properties -> C/C++ -> Advanced -> Forced Include File.
Later on, or better, ASAP, remove the force include and work your way through the warnings, since most of them are quite easy to fix (size_t instead if int, etc).

OTHER TIPS

Perhaps you could create new code in separate DLLs or Libraries. That way you can enforce your higher warning level (and I would say go for /W4 and be prepared to turn off a few of MS's dafter warnings rather than settle for /W3) without having to wade through 1000s of warnings from the old code.

Then you can start working on cleaning up the old code, a bit at a time, when there is time -- making sure you have suitable unit tests in place to avoid breaking it by accident of course.

you may not like the answer...

remove the warnings by correcting the issues.

i'm very picky about warning levels; even i ignore warnings which i don't need to correct, especially when the warning level is high and build times are high. meanwhile, new ones slip in (in large codebases). removing them incrementally doesn't work very well, in my experience -- they tend to get ignored if the noise is too high, or it is not enforced.

you need to reduce the warning noise so people can see the warnings they add (at the warning level you desire).

to reach the compliance level you want/need, make it a priority.

if you don't know whether the conversions/comparisons are valid, you can always use a template function with an error action (assert, throw, log) to perform the logic when in doubt.

it can be a slow/tedious process, but it's also a good way to learn the codebase.

i typically start at the libraries highest in the tree, or those which are reused most often. once a library meets a standard, maintain that standard.

If you are going to make code modifications as a result of the new stricter warning level, write adequate tests that protect against introducing new problems/bugs. Write the tests using the new warning level. Do this before you start to change the codebase and verify correct functionality. Then you can rerun the updated code against the same test case.

I would use an incremental approach.

The first step would be to modify the old files and add the required pragma action to deactivate the warning in the code.

The second step is to build a commit-hook that will refuse any committed file that contain the specific pragma pattern that discards all those "old" warnings.

This means that any modified file should be warning free.

However, let us be frank, developers always find ways to game the system.

My approach has been to go with the highest warning level you can and fix all the warnings that come up - you may even find some bugs in the process.

You should set this up using vsprops files so that all projects are compiled with the same warning level and any changes you make to these settings change in all projects.

A more incremental approach is to go with the highest warning level you can and then disable almost all warnings, leaving you with only a small number of warnings to consider at once - fix those and then switch on another warning, and so on until you are free of warnings.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top