Hi Petr,

Great post! I agree with your suggestions, I’ll only comment some of the warnings.

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.

Yes, we should remove them, but you could filter all such warnings from the Gnutella package.

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.

value_ is non-pointer member. Could you look more into that please?

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.

I’d suggest that we tag all classes with one of: REFERENCE_OBJECT, VALUE_OBJECT, STATIC_HELPER. Maybe we could also create a custom Doxygen command (seems to be possible to be defined in Doxyfile) and tag also the docs.

Effective C++ Item 12 field member “Some class member” not initialized (preferable to assignment in constructors)

g++ reports that too - all of them should be fixed now (except for Protocols/BitTorrent and Utils/ObjectDispatcher which are probably fixed in other branches already).

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

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

What’s the exact meaning of this warning? That the forward declaration is unnecessary because the the class is fully specified already?

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.

g++ does not seem to report that. We should probably assert that p.resultSet.size() < = 0xFF. If we use a static_cast the warning should go away.

Declaration of “some variable” shadows a member of ‘this’

We should enforce the memberName_ naming convention.

function “QDialog::done(int)” is hidden by “UIs::ManagePacketsDialog::done” — virtual function override intended?

g++ reported that too - I’ve fixed it in calitko,141

Please fix as many warnings as you can in your branch and I’ll merge all your revisions at once.

Maybe you could also update your calitko-dev branch and also fix all remaining BitTorrent warnings. I’ll do the same to fix the warnings in Utils::ObjectDispatcher. We’ll very soon have clean builds ;-)

Best regards,

Peter