Dear all,

I’ve recently added extra warnings for ICC (Intel C/C++ compiler) and compiled my latest calitko branch (developers/s3rvac/calitko,142) with this compiler (using our compilation.py script). There are some warnings which I’d like to discuss (I’ll follow the following layout: “name” of the warning, places of appearance (example[s]), suggestion how to fix it or why we should ignore it). The total count of warnings is only orientation.

using-declaration ignored — it refers to the current namespace
Appears only in the Gnutella package (79 total).

./Gnutella/NodeInfo.h(36): warning #780: using-declaration ignored -- it refers to the current namespace
using Gnutella::NodeType;


These using declarations are superfluous, so we should remove them.

Effective C++ Item 1 prefer const and inline to #define
Everywhere (18967 total).

Protocols/BitTorrent/Bencoding/BString.h(24): warning #2012: Effective C++ Item 1 prefer const and inline to #define
#define PROTOCOLS__BIT_TORRENT__BENCODING__B_STRING_H


Since this warning appears with each preprocessor directive, it’s useless. I think we all know that macros are “evil”, but there are situation where they’re handy and can’t be replaced with const/inline. My suggestion is to ignore this warning.

Effective C++ Item 4 prefer C++ style comments
Gnutella,Http and UIs mostly (360 total).

Http/Testing/HeaderTest.cpp(204): warning #2015: Effective C++ Item 4 prefer C++ style comments
CPPUNIT_ASSERT (m_p->isValid() == false ); /* Does not work */
--
UIs/Searching/SearchModel.h(40): warning #2015: Effective C++ Item 4 prefer C++ style comments
bitrate("8 bps"), /*parent(NULL), */hosts(), showEmphasized (false)


There are no problems, this is just a matter of style, so we could some of them replace with “C++” comments, but I’d suggest to ignore these.

Effective C++ Item 6 missing delete of member pointer member “Some variable” in destructor
Nearly everywhere (777 total).

Protocols/BitTorrent/Bencoding/BString.h(58): warning #2017: Effective C++ Item 6 missing delete of member pointer member "Protocols::BitTorrent::Bencoding::BString::value_" in destructor
{}


I don’t really get (understand) this warning - it appears also on places where there is no pointer member variable… I’d understand it if it was only in dtors of classes with some pointer member variable.

Effective C++ Item 11 declare a copy constructor and assignment operator for PrivateData
Gnutella, Http and CalitkoMocks mostly (734 total).

./Http/HeaderReader.h(65): warning #2021: Effective C++ Item 11 declare a copy constructor and assignment operator for Private
} p;


I first thought that this warning was useless, but then I saw that in classes where we don’t use QSharedData (or some other class that have defined copy ctor/assignment operator) for storing private data we don’t have our own copy ctor/assignment operator defined, but there are pointers in the PrivateData classes. This could lead to problems (copy ctor/assignment operator generated by a compiler performs only shalow copy, not deep copy). My suggestion is to either use our REFERENCE_OBJECT macro to disable copy ctor/assignment operator, or use QSharedData instead of a struct or define our own copy ctor/assignment operator.

Effective C++ Item 12 field member “Some class member” not initialized (preferable to assignment in constructors)
Gnutella, CalitkoMocks and generated files mostly (220 total).

Gnutella/Bootstrapping/NodeCache.cpp(81): warning #2022: Effective C++ Item 12 field member "Gnutella::Bootstrapping::NodeCachePrivate::availabilityList" not initialized (preferable to assignment in constructors)
NodeCachePrivate() : nodeTable(), completeList() {}


This warning is similar to the g++ “…should be initialized in the member initialization list”, so we should ignore it if it’s in some PrivateData class and fix it otherwise (most of these warnings are already fixed).

NULL reference is not allowed
CalitkoMocks (6 total).

./Utils/CalitkoMocks/BasicTypes.h(135): warning #327: NULL reference is not allowed
T & DefaultValue <T &>::value = *reinterpret_cast <T *> (0); // HACK!!!


Peter will fix this (we’ve already discussed this one).

Declaration does not declare anything
./Gnutella/LocalPeer.h:36.

./Gnutella/LocalPeer.h(36): warning #64: declaration does not declare anything
class UIs::Searching::SearchModel;


Superfluous, remove it (there is even written: “// get away with these:”).

conversion from “int” to “uchar={unsigned char}” may lose significant bits
Gnutella, UIs, Utils::Uri (115 total).

./Gnutella/Packets/QueryHits.h(160): warning #810: conversion from "int" to "uchar={unsigned char}" may lose significant bits
uchar numberOfHits() const { return p.resultSet.size(); }


We should check whether there could be no range problems.

Parameter “some variable” was never referenced
Gnutella, UIs and Utils (25 total).

UIs/DownloadProgressBar.cpp(57): warning #869: parameter "start" was never referenced
void DownloadProgressBar::removeRequestedRange (int start, int end)


This is similar to the g++ “unused parameter” warning - we have already discussed this one (we should keep this warning since there is mostly some work needed).

Variable “variable name” was declared but never referenced
Variable “variable name” was set but never used
CalitkoMocks mostly (16 total).

Utils/CalitkoMocks/Testing/CallDriverTest.cpp(119): warning #177: variable "expectationDriver" was declared but never referenced
expectationDriver = driver.willCall (pair);


This is similar to the g++ “unused variable” warning - see the previous warning.

Controlling expression is constant
Protocols/BitTorrent/Packets/BadPacket.cpp (6 total)
Utils/Uri.cpp (1 total)

Protocols/BitTorrent/Packets/BadPacket.cpp(54): warning #279: controlling expression is constant
Q_ASSERT (false && "Modifying header fields of BadPackets not allowed!");


We’ve agreed to change the form of these assertions to Q_ASSERT (false); // Modifying header fields of BadPackets not allowed!. I think it should be fixed in the Peter’s dev branch already.

Declaration of “some variable” shadows a member of ‘this’
Protocols package, UIs package and CalitkoMocks (15 total)

Protocols/Generics/TcpSocketBuffer.cpp(167): warning #1944: declaration of "readSize" shadows a member of 'this'
the shadowed declaration is at line 93 of "Protocols/Generics/TcpSocketBuffer.h"
int readSize = 0;


We should check all of these and give these variables another name (it could be misleading).

Declaration hides variable “variable name”
Gnutella package, Http/Header.cpp and Protocols/Generics/PacketSerializer.cpp (8 total).

Protocols/Generics/PacketSerializer.cpp(61): warning #1599: declaration hides variable "header" (declared at line 57)
QByteArray header = transport->read (headerLength);


This is more or less the same warning as the previous one (we should give these variables another name, too).

function “QDialog::done(int)” is hidden by “UIs::ManagePacketsDialog::done” — virtual function override intended?
UIs/PacketDumpTreeView.h:55 (1 total)

UIs/PacketDumpTreeView.h(55): warning #1125: function "QDialog::done(int)" is hidden by "UIs::ManagePacketsDialog::done" -- virtual function override intended?
void done();


If virtual function override was intended, we should change the signature of that function. If not, we should change the name of the function.

These are all warnings that were produced by the Intel compiler (uff :-)). Feel free to write down your suggestions! You can also check the compilation log (see attached file).

Regards,

Petr