On 01/12/2015 06:00 PM, Chuck Rolke wrote:
Visual Studio 2010 x64 generates five new warnings compiling LossyLvq code.
The code may be correct and the compiler is known to warn on correct code.
That said, if the code could be reorganized to avoid the warnings maybe
Coverity would stop complaining as well.

[...]

3>C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\memory(931):
        warning C4150: deletion of pointer to incomplete type 
'qpid::broker::MessageMap'; no destructor called
3>          
D:\Users\crolke\git\rh-qpid\qpid\cpp\src\qpid/broker/QueueCursor.h(65) :
       see declaration of 'qpid::broker::MessageMap'
3>          C:\Program Files (x86)\Microsoft Visual Studio 
10.0\VC\include\memory(930) : while compiling class template member function 
'std::auto_ptr<_Ty>::~auto_ptr(void)'
3>          with
3>          [
3>              _Ty=qpid::broker::MessageMap
3>          ]
3>          ..\..\cpp\src\qpid\broker\LossyLvq.cpp(27) : see reference to class 
template instantiation 'std::auto_ptr<_Ty>' being compiled
3>          with
3>          [
3>              _Ty=qpid::broker::MessageMap
3>          ]
3>  QueueFactory.cpp

That one seems fair, and I've checked in what I believe is a fix for that.

3>D:\Users\crolke\git\rh-qpid\qpid\cpp\src\qpid/broker/LossyLvq.h(38): warning 
C4250: 'qpid::broker::LossyLvq' : inherits 
'qpid::broker::Lvq::qpid::broker::Lvq::push' via dominance
3>          D:\Users\crolke\git\rh-qpid\qpid\cpp\src\qpid/broker/Lvq.h(39) : 
see declaration of 'qpid::broker::Lvq::push'

3>D:\Users\crolke\git\rh-qpid\qpid\cpp\src\qpid/broker/LossyLvq.h(38): warning 
C4250: 'qpid::broker::LossyLvq' : inherits 
'qpid::broker::LossyQueue::qpid::broker::LossyQueue::checkDepth' via dominance
3>          
D:\Users\crolke\git\rh-qpid\qpid\cpp\src\qpid/broker/LossyQueue.h(36) : see 
declaration of 'qpid::broker::LossyQueue::checkDepth'

Those are exactly the methods I expect to be inherited. LossyQueue alters the implementation of checkDepth() to support the 'ring' behaviour, Lvq alters the implementation of push() to support the lvq behaviour. What I wanted was a Queue type that inherited both these behaviours. To me, this is exactly the use case virtual inheritance was intended for.

I could of course organise the code so as not to need to use virtual inheritance, but I'd rather only do that if there was a good reason.

With respect to the last two warnings above, does the attached patch resolve them? (This overrides both methods in the most derived class and explicitly chooses the implementation to use).

diff --git a/qpid/cpp/src/qpid/broker/LossyLvq.cpp b/qpid/cpp/src/qpid/broker/LossyLvq.cpp
index f59ecc1..0d773bc 100644
--- a/qpid/cpp/src/qpid/broker/LossyLvq.cpp
+++ b/qpid/cpp/src/qpid/broker/LossyLvq.cpp
@@ -27,4 +27,14 @@ namespace broker {
 LossyLvq::LossyLvq(const std::string& n, std::auto_ptr<MessageMap> m, const QueueSettings& s, MessageStore* const ms, management::Manageable* p, Broker* b)
     : Queue(n, s, ms, p, b), Lvq(n, m, s, ms, p, b), LossyQueue(n, s, ms, p, b) {}
 
+void LossyLvq::push(Message& message, bool isRecovery)
+{
+    Lvq::push(message, isRecovery);
+}
+
+bool LossyLvq::checkDepth(const QueueDepth& increment, const Message& message)
+{
+    return LossyQueue::checkDepth(increment, message);
+}
+
 }} // namespace qpid::broker
diff --git a/qpid/cpp/src/qpid/broker/LossyLvq.h b/qpid/cpp/src/qpid/broker/LossyLvq.h
index e0a266a..1ccc64a 100644
--- a/qpid/cpp/src/qpid/broker/LossyLvq.h
+++ b/qpid/cpp/src/qpid/broker/LossyLvq.h
@@ -35,6 +35,8 @@ class LossyLvq : public Lvq, public LossyQueue
 {
   public:
     LossyLvq(const std::string&, std::auto_ptr<MessageMap>, const QueueSettings&, MessageStore* const, management::Manageable*, Broker*);
+    void push(Message& msg, bool isRecovery=false);
+    bool checkDepth(const QueueDepth& increment, const Message&);
 };
 }} // namespace qpid::broker
 
diff --git a/qpid/cpp/src/qpid/broker/LossyQueue.h b/qpid/cpp/src/qpid/broker/LossyQueue.h
index 705865f..02ad127 100644
--- a/qpid/cpp/src/qpid/broker/LossyQueue.h
+++ b/qpid/cpp/src/qpid/broker/LossyQueue.h
@@ -33,7 +33,7 @@ class LossyQueue : public virtual Queue
 {
   public:
     LossyQueue(const std::string&, const QueueSettings&, MessageStore* const, management::Manageable*, Broker*);
-    bool checkDepth(const QueueDepth& increment, const Message&);
+    virtual bool checkDepth(const QueueDepth& increment, const Message&);
   private:
 };
 }} // namespace qpid::broker
diff --git a/qpid/cpp/src/qpid/broker/Lvq.h b/qpid/cpp/src/qpid/broker/Lvq.h
index 26ba2b4..55837ed 100644
--- a/qpid/cpp/src/qpid/broker/Lvq.h
+++ b/qpid/cpp/src/qpid/broker/Lvq.h
@@ -36,7 +36,7 @@ class Lvq : public virtual Queue
 {
   public:
     Lvq(const std::string&, std::auto_ptr<MessageMap>, const QueueSettings&, MessageStore* const, management::Manageable*, Broker*);
-    void push(Message& msg, bool isRecovery=false);
+    virtual void push(Message& msg, bool isRecovery=false);
   private:
     MessageMap& messageMap;
 };

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to