Hi Petr,

Revno 134 fixes the “..hides…” warning, as well as a few others from the log you provided. I ignored the warnings coming from the old Gnutella package as it will be refactored anyway.

I’d like to discuss how should we solve some of the remaining icpc and g++ warnings:

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

I do like the trick because if the assertion ever fails we’ll also have the message printed but we in any case will also get the filename and line printed, so using that notation:

Q_ASSERT (false); // Modifying header fields of BadPackets not allowed!

will give us the same information but would require us to have a look at the source code, which we will have to do anyway. It’ll be great if we got a clean compile with no warnings. If we have plenty of them, we would get into habit of ignoring them even though they could be helpful at times. So in the sake of clean compiles I’d suggest that we the latter variant. What would you say?

Utils/CalitkoMocks/Testing/DelayedCallDriverTest.cpp(62): warning #327: NULL reference is not allowed
fooType> driver (*(ExpectationDriverFoo *) 0, delayedFoo);
^

That’s truly hacky, so I’d better fix it :-) It does work because these tests never try to dereference the null reference. I just wanted to spare myself a few more initialization lines.

I compiled with gcc using -std=c++98 -pedantic and the rest of the extra warnings flags:

./Protocols/Generics/PacketBase.h: In constructor ‘Protocols::Generics::PacketBase::Data::Data()’:
./Protocols/Generics/PacketBase.h:110: warning: ‘Protocols::Generics::PacketBase::Data::rawHeader’ should be initialized in the member initialization list
...

At that place we have not declared a default ctor and we don’t really need one. Should we define a default ctor just to satisfy the compiler? If we want “strict” code and clean compiles, then I guess we should…

./Utils/CalitkoMocks/BoundFunction.h:168: warning: use of old-style cast

I’ll fix that!

./Protocols/Generics/GenericSession.h: At global scope:
./Protocols/Generics/GenericSession.h:102: warning: ‘class Protocols::Generics::GenericSession’ has pointer data members
./Protocols/Generics/GenericSession.h:102: warning: but does not override ‘Protocols::Generics::GenericSession(const Protocols::Generics::GenericSession&)’
./Protocols/Generics/GenericSession.h:102: warning: or ‘operator=(const Protocols::Generics::GenericSession&)’

That’s actually a quite common thing, so I thought about making a macro for that. What about adding REFERENCE_OBJECT (GenericSession); in the private part of a class, which would define private copy-ctor and assignment operators (Qt have Q_DISABLE_COPY but it’s not part of the api, so we’d better define our own).

I was also thinking, in some classes we could write VALUE_OBJECT (Packet) in the public part, which would declare copy-ctor, assignment operator and the comparison operators, which would need to be defined in the cpp file. We would need to write less, but may make the class more obscure… although using the terms ValueObject and ReferenceObject might even improve readability?

Actually these are the kind of warnings that produce probably over 90% of all calitko warnings, which means we could very soon get a clean compile :-).

Regards,

Peter