Help reducing warnings in source code

[Seismo]

I know, to eradicate the warnings might be slightly from cosmetic perspective. But i am sure i have seen some serious issues which are able to cause some strange errors. In general i am programming C++ only, but i would like to help you fighting against this warnings, if you are interested. May you have other ways of doing that... Please let me know.

[Per]

I see no warnings here with gcc and -Werror. What compiler are you using?

[Samowar]

MSVC 2008 Pro. Apparently this does produce warnings. See here

[Seismo]

And this was just warning level 3. With level 4 for (Warzone2100 only) i get 230 warnings. With all set to level 4 i get 243 warnings (excluded the 3rd party libs).

[Samowar]

Could you post some of these warnings here?

btw, the only warnings I get when compiling on Linux are of the type:

/usr/bin/xgettext: Warnung: Typ der Datei »data/base/messages/messages.rmsg« mit Suffix »rmsg« ist unbekannt; C wird versucht

- but that's nothing to do with the compiler itself.

[Per]

It certainly would be interesting to reduce the number of warnings. So patches to fix them would be appreciated.

[Seismo]

Here the result for compiled autorevision.cpp with level 4 warnings on:

Code: Select all

1>------ Erstellen gestartet: Projekt: autorevision, Konfiguration: Release Win32 ------
1>Kompilieren...
1>autorevision.cpp
1>.\autorevision.cpp(144) : warning C4512: 'RevisionExtractor': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(116): Siehe Deklaration von 'RevisionExtractor'
1>.\autorevision.cpp(166) : warning C4512: 'RevSVNVersionQuery': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(154): Siehe Deklaration von 'RevSVNVersionQuery'
1>.\autorevision.cpp(180) : warning C4512: 'RevSVNQuery': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(168): Siehe Deklaration von 'RevSVNQuery'
1>.\autorevision.cpp(194) : warning C4512: 'RevFileParse': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(182): Siehe Deklaration von 'RevFileParse'
1>.\autorevision.cpp(211) : warning C4512: 'RevGitSVNQuery': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(196): Siehe Deklaration von 'RevGitSVNQuery'
1>.\autorevision.cpp(225) : warning C4512: 'RevConfigFile': Zuweisungsoperator konnte nicht generiert werden
1>        .\autorevision.cpp(213): Siehe Deklaration von 'RevConfigFile'
1>.\autorevision.cpp(585) : warning C4800: 'int': Variable wird auf booleschen Wert ('True' oder 'False') gesetzt (Auswirkungen auf Leistungsverhalten möglich)
1>Das Buildprotokoll wurde unter "file://d:\Programming\Warzone2100\build_tools\autorevision\Release\BuildLog.htm" gespeichert.
1>autorevision - 0 Fehler, 7 Warnung(en)
========== Erstellen: 1 erfolgreich, Fehler bei 0, 0 aktuell, 0 übersprungen ==========

warning C4512:http://msdn.microsoft.com/en-us/library ... S.80).aspx
warning C4800:http://msdn.microsoft.com/en-us/library ... S.71).aspx

autorevision.cpp(144) : warning C4512: 'RevisionExtractor': Possible Solution:

Code: Select all

class RevisionExtractor
{
    public:
        /** Constructor
         *  \param s successor of this instantation. If this instantation fails
         *           retrieving the revision, the successor pointed to by \c s,
         *           will attempt to do it instead.
         */
        RevisionExtractor(RevisionExtractor* s = NULL) :
            _successor(s)
        {}

		// Explicitly define an assignment operator for the class to avoid C4512 warning
		RevisionExtractor & operator=( const RevisionExtractor & ) 
		{}

        virtual ~RevisionExtractor()
        {}

        /** This function will perform the actual work of extracting the
         *  revision information.
         *
         *  RevisionExtractor::extractRevision can be used by subclasses to
         *  delegate the task to its successor if it cannot succeed itself.
         *
         *  For that purpose simply use a line like:
         *    "return extractRevision(revision, date, wc_uri);"
         */
        virtual bool extractRevision(RevisionInformation& rev_info) = 0;

    private:
        // Declare the pointer itself const; not whatever it points to
        RevisionExtractor* const _successor;
};

[Samowar]

warning C4512:http://msdn.microsoft.com/en-us/library ... S.80).aspx

I don't know if it's worth "fixing" all these; they seem not to have any impact at all (see this discussion). Of course it would be "cheating", but an easier way to fix these warnings would be to include

Code: Select all

#ifdef _MSC_VER //stop MSVC++ from complaining
#pragma warning( disable : 4512 )
#endif //_MSC_VER

in the header. Fixing the bool<->int warning, however, seems legitimate to me.

[Seismo]

In this case you may right, why the heck should anyone do that. I know from several other projects that they ignore that whole the time. If it´s running... time is money... and several other reasons they have.

May this was not the best example. So if you want a better example that ignoring warnings can be dangerous: For e. g. have a closer look to exchndl.c. The code probably *seems* to work because CHAR and _T("A String") happen to be the same size and are passed as parameters using the same mechanism -- on your system. But on a unicode machine CHAR wont fit into WCHAR without correct conversion.

In my opinion this is the root cause for a lot upcomming issues. And the source code is obviously having a bad hack status instead of a (new)developer friendly well formated one. I was working in several Projects. Warnings counted there as errors. Or when the weekly build popped up a (level 4) warning you had to pay at least 5 €uro per warning in the team coffee collection.

I would like to state the following:
1. I can do patching this, its a good practice to refresh my rusty c (not c++) knowledge
2. Level 4 Warnings ->243, Level 3 Warnings ->230 I would like to reduce to level 3 again, but can you see which one of the 230 issues are similar weak as we see in my first example?
3. I am not here to do finger pointing, i am a detail loving nerd guessing that warnings in that mass might not help to provide strong and hardened software.

[Samowar]

I was working in several Projects. Warnings counted there as errors.

So they do here. The default switches for gcc are -Wall -Werror (and some more) and it compiles just fine and without warnings. However, perhaps some more switches could be turned on. Turning on -Wextra does produce warnings/errors.

[Buginator]

It certainly would be interesting to reduce the number of warnings. So patches to fix them would be appreciated.

It would touch a ton of code.
The main issue is we are compiling with a C++ compiler, and it is spitting out lots of warnings that wouldn't otherwise show up.
We have added a bunch of pragmas to shut it up, but there are still tons of warnings.

Not really sure if doing a ton of casts is worth it...

[Seismo]

I finished it now with 25 Warnings left, comming from Bison/Flex and some libraries. I suppess 2 warnings to reduce warned issues which are in combination of Flex/Bison/GLee with MSVC. Without supressing i have 95 (often repeated) warnings left. So, not all is supressed i also fixed up some unicode issues... but look for yourself. You will find a first patch attached. Please let me know your impressions with it.

EDIT: The files are here now: viewtopic.php?f=6&t=4727&start=15#p47686

[Buginator]

That is one huge patch.
Looks like you ran the code through a beautifier as well?

Like I said though, if we apply this patch, it would ruin the revision history, make diffs / merges harder, and I am not sure we want that.

[Seismo]

Oh, ok, no help wanted, but it was a try.

[Samowar]

if we apply this patch, it would ruin the revision history, make diffs / merges harder, and I am not sure we want that.

Why is this? If you just apply it to svn as a normal bugfixing revision? I think compiler warnings, even if they only occur on some platforms, should be considered bugs and either fixed or shut up by a pragma.