Ken Foskey wrote:
On Mon, 2005-08-29 at 17:59 +0200, Stephan Bergmann wrote:


Warning free code is a basic precondition for robust industrial quality code. This as well is current scientific knowledge as the result of the personal experience of Hamburg-internal engineers, as well as the opinion of some OpenOffice.org developers like e.g. Ken Foskey.


So I would actually recommend against an all out warnings push unless
everyone is VERY clear the objective is to highlight bugs not to remove
warnings.  The difference in objectives is very important.

Not sure I understand you correctly.

1 The main objective is to get the complete code base to build successfully with wall=1 werror=1. Once we have achieved that, a programmer can easily find new bugs he introduces by building with wall=1 werror=1 (which can also become the default build mode then).

2 To reach that main objective, we have to remove warnings from the current code base. There are to cases:

2a The warning indicates an error in the code. We will fix that error (which is a positive side effect of the efforts spent on this CWS; also, we should probably consider to back-port any serious errors we find to some pre--OOo-3 branch).

2b The warning is a spurious one. However, to reach the main objective, we have to make it go away anyway, by modifying the code base in some way or other. This (as well as deciding whether the occurence of a warning is case 2a or 2b) is a delicate task, to be sure. ("Hubris" you may cry, and, yes, you probably are right. However, "if in dout, leave it alone" is surrender in front of the code mass, a mass that we have to keep agile and healthy, though, for a living and/or for fun. "If in doubt, write a test" is a motto I sympathize with more.)

-Stephan

** If in doubt leave it alone. **

I am also concerned that then process will become a template fix.

if( fp = fopen( "file", "r" )) {

can become:

if( (fp = fopen( "file", "r")) ) {

If I assign someone to clean up the error, say a junior programmer
because it is menial, and we have this code:

if( x = 4 ) {

They may very well apply the 'template' solution hiding a useful
warning.

if( (x = 4) ) {

That useful warning being removed is WORSE than the problem of many
warnings.  This gets really tricky when you get into essoteric C++
warnings.

The objective of the push should be to highlight bugs by removing as
many warnings with obvious solutions as possible.  If in doubt leave the
warning!

As Pavel has hinted it is possibly better to work through one warning
class at a time, eg assignment bugs. This can be discussed so that the
approach to each is correct, eg don't just bracket them.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to