From 493e23665cb610a9c31e5511342b66891c1a765b Mon Sep 17 00:00:00 2001
From: Stephen Sinclair <stephen.sinclair@inria.cl>
Date: Thu, 31 Aug 2017 15:15:19 -0300
Subject: [PATCH] Fix extraneous size check in queue.pop() and document queue
 code.

---
 RtMidi.cpp | 50 +++++++++++++++++++++++++++++++-------------------
 RtMidi.h   |  5 +++--
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/RtMidi.cpp b/RtMidi.cpp
index 78c7d1ba..8ffe634a 100644
--- a/RtMidi.cpp
+++ b/RtMidi.cpp
@@ -344,8 +344,6 @@ double MidiInApi :: getMessage( std::vector<unsigned char> *message )
     return 0.0;
   }
 
-  if ( inputData_.queue.size == 0 ) return 0.0;
-
   double timeStamp;
   if (!inputData_.queue.pop(message, &timeStamp))
     return 0.0;
@@ -353,19 +351,34 @@ double MidiInApi :: getMessage( std::vector<unsigned char> *message )
   return timeStamp;
 }
 
-bool MidiInApi::MidiQueue::push(const MidiInApi::MidiMessage& msg)
+unsigned int MidiInApi::MidiQueue::size(unsigned int *__back,
+					unsigned int *__front)
 {
-  // As long as we haven't reached our queue size limit, push the message.
-  unsigned int _back = back;
-  unsigned int _front = front;
-  unsigned int size;
-
+  // Access back/front members exactly once and make stack copies for
+  // size calculation
+  unsigned int _back = back, _front = front, _size;
   if (_back >= _front)
-    size = _back - _front;
+    _size = _back - _front;
   else
-    size = ringSize - _front + _back;
+    _size = ringSize - _front + _back;
+
+  // Return copies of back/front so no new and unsynchronized accesses
+  // to member variables are needed.
+  if (__back) *__back = _back;
+  if (__front) *__front = _front;
+  return _size;
+}
+
+// As long as we haven't reached our queue size limit, push the message.
+bool MidiInApi::MidiQueue::push(const MidiInApi::MidiMessage& msg)
+{
+  // Local stack copies of front/back
+  unsigned int _back, _front, _size;
+
+  // Get back/front indexes exactly once and calculate current size
+  _size = size(&_back, &_front);
 
-  if ( size < ringSize-1 )
+  if ( _size < ringSize-1 )
   {
     ring[_back] = msg;
     back = (back+1)%ringSize;
@@ -377,21 +390,20 @@ bool MidiInApi::MidiQueue::push(const MidiInApi::MidiMessage& msg)
 
 bool MidiInApi::MidiQueue::pop(std::vector<unsigned char> *msg, double* timeStamp)
 {
-  unsigned int _back = back;
-  unsigned int _front = front;
-  unsigned int size;
+  // Local stack copies of front/back
+  unsigned int _back, _front, _size;
 
-  if (_back >= _front)
-    size = _back - _front;
-  else
-    size = ringSize - _front + _back;
+  // Get back/front indexes exactly once and calculate current size
+  _size = size(&_back, &_front);
 
-  if (size == 0)
+  if (_size == 0)
     return false;
 
   // Copy queued message to the vector pointer argument and then "pop" it.
   msg->assign( ring[_front].bytes.begin(), ring[_front].bytes.end() );
   *timeStamp = ring[_front].timeStamp;
+
+  // Update front
   front = (front+1)%ringSize;
   return true;
 }
diff --git a/RtMidi.h b/RtMidi.h
index 2f4ebd54..83252125 100644
--- a/RtMidi.h
+++ b/RtMidi.h
@@ -516,15 +516,16 @@ class MidiInApi : public MidiApi
   struct MidiQueue {
     unsigned int front;
     unsigned int back;
-    unsigned int size;
     unsigned int ringSize;
     MidiMessage *ring;
 
     // Default constructor.
   MidiQueue()
-  :front(0), back(0), size(0), ringSize(0), ring(0) {}
+  :front(0), back(0), ringSize(0), ring(0) {}
     bool push(const MidiMessage&);
     bool pop(std::vector<unsigned char>*, double*);
+    unsigned int size(unsigned int *back=0,
+		      unsigned int *front=0);
   };
 
   // The RtMidiInData structure is used to pass private class data to