Fix a few memory issues found with ASan (#6843)

* Fix LADSPA effects memory leak

* Fix buffer overflow in PianoView

* Avoid using invalid iterators in AutomationClip

* Fix memory leaks in SimpleTextFloat

* Handle potential case where QMap::lowerBound(...) returns end iterator

* Implement suggestions from review

plugins/LadspaEffect/caps/Descriptor.h

@@ -53,7 +53,7 @@ class DescriptorStub
 				PortCount = 0;
 			}
 
-		~DescriptorStub()
+		virtual ~DescriptorStub()
 			{
 				if (PortCount)
 				{
@@ -87,6 +87,7 @@ class Descriptor
 
 	public:
 		Descriptor() { setup(); }
+		~Descriptor() override = default;
 		void setup();
 
 		void autogen() 

src/core/AutomationClip.cpp

@@ -1106,31 +1106,23 @@ void AutomationClip::generateTangents(timeMap::iterator it, int numToGenerate)
 {
 	QMutexLocker m(&m_clipMutex);
 
-	if( m_timeMap.size() < 2 && numToGenerate > 0 )
+	for (int i = 0; i < numToGenerate && it != m_timeMap.end(); ++i, ++it)
 	{
-		it.value().setInTangent(0);
-		it.value().setOutTangent(0);
-		return;
-	}
-
-	for( int i = 0; i < numToGenerate; i++ )
-	{
-		if( it == m_timeMap.begin() )
+		if (it + 1 == m_timeMap.end())
+		{
+			// Previously, the last value's tangent was always set to 0. That logic was kept for both tangents
+			// of the last node
+			it.value().setInTangent(0);
+			it.value().setOutTangent(0);
+		}
+		else if (it == m_timeMap.begin())
 		{
 			// On the first node there's no curve behind it, so we will only calculate the outTangent
 			// and inTangent will be set to 0.
 			float tangent = (INVAL(it + 1) - OUTVAL(it)) / (POS(it + 1) - POS(it));
 			it.value().setInTangent(0);
 			it.value().setOutTangent(tangent);
 		}
-		else if( it+1 == m_timeMap.end() )
-		{
-			// Previously, the last value's tangent was always set to 0. That logic was kept for both tangents
-			// of the last node
-			it.value().setInTangent(0);
-			it.value().setOutTangent(0);
-			return;
-		}
 		else
 		{
 			// When we are in a node that is in the middle of two other nodes, we need to check if we
@@ -1159,7 +1151,6 @@ void AutomationClip::generateTangents(timeMap::iterator it, int numToGenerate)
 				it.value().setOutTangent(outTangent);
 			}
 		}
-		it++;
 	}
 }
 

src/gui/instrument/PianoView.cpp

@@ -322,70 +322,65 @@ void PianoView::modelChanged()
 
 
 
-// gets the key from the given mouse-position
+// Gets the key from the given mouse position
 /*! \brief Get the key from the mouse position in the piano display
  *
- *  First we determine it roughly by the position of the point given in
- *  white key widths from our start.  We then add in any black keys that
- *  might have been skipped over (they take a key number, but no 'white
- *  key' space).  We then add in our starting key number.
- *
- *  We then determine whether it was a black key that was pressed by
- *  checking whether it was within the vertical range of black keys.
- *  Black keys sit exactly between white keys on this keyboard, so
- *  we then shift the note down or up if we were in the left or right
- *  half of the white note.  We only do this, of course, if the white
- *  note has a black key on that side, so to speak.
- *
- *  This function returns const because there is a linear mapping from
- *  the point given to the key returned that never changes.
- *
- *  \param _p The point that the mouse was pressed.
+ *  \param p The point that the mouse was pressed.
  */
-int PianoView::getKeyFromMouse( const QPoint & _p ) const
+int PianoView::getKeyFromMouse(const QPoint& p) const
 {
-	int offset = _p.x() % PW_WHITE_KEY_WIDTH;
-	if( offset < 0 ) offset += PW_WHITE_KEY_WIDTH;
-	int key_num = ( _p.x() - offset) / PW_WHITE_KEY_WIDTH;
+	// The left-most key visible in the piano display is always white
+	const int startingWhiteKey = m_pianoScroll->value();
 
-	for( int i = 0; i <= key_num; ++i )
-	{
-		if ( Piano::isBlackKey( m_startKey+i ) )
-		{
-			++key_num;
-		}
-	}
-	for( int i = 0; i >= key_num; --i )
-	{
-		if ( Piano::isBlackKey( m_startKey+i ) )
-		{
-			--key_num;
-		}
-	}
+	// Adjust the mouse x position as if x == 0 was the left side of the lowest key
+	const int adjX = p.x() + (startingWhiteKey * PW_WHITE_KEY_WIDTH);
+
+	// Can early return for notes too low
+	if (adjX <= 0) { return 0; }
+
+	// Now we can calculate the key number (in only white keys) and the octave
+	const int whiteKey = adjX / PW_WHITE_KEY_WIDTH;
+	const int octave = whiteKey / Piano::WhiteKeysPerOctave;
 
-	key_num += m_startKey;
+	// Calculate for full octaves
+	int key = octave * KeysPerOctave;
 
-	// is it a black key?
-	if( _p.y() < PIANO_BASE + PW_BLACK_KEY_HEIGHT )
+	// Adjust for white notes in the current octave
+	// (WhiteKeys maps each white key to the number of notes to their left in the octave)
+	key += static_cast<int>(WhiteKeys[whiteKey % Piano::WhiteKeysPerOctave]);
+
+	// Might be a black key, which would require further adjustment
+	if (p.y() < PIANO_BASE + PW_BLACK_KEY_HEIGHT)
 	{
-		// then do extra checking whether the mouse-cursor is over
-		// a black key
-		if( key_num > 0 && Piano::isBlackKey( key_num-1 ) &&
-			offset <= ( PW_WHITE_KEY_WIDTH / 2 ) -
-					( PW_BLACK_KEY_WIDTH / 2 ) )
+		// Maps white keys to neighboring black keys
+		static constexpr std::array neighboringKeyMap {
+			std::pair{ 0, 1 }, // C --> no B#; C#
+			std::pair{ 1, 1 }, // D --> C#; D#
+			std::pair{ 1, 0 }, // E --> D#; no E#
+			std::pair{ 0, 1 }, // F --> no E#; F#
+			std::pair{ 1, 1 }, // G --> F#; G#
+			std::pair{ 1, 1 }, // A --> G#; A#
+			std::pair{ 1, 0 }, // B --> A#; no B#
+		};
+
+		const auto neighboringBlackKeys = neighboringKeyMap[whiteKey % Piano::WhiteKeysPerOctave];
+		const int offset = adjX - (whiteKey * PW_WHITE_KEY_WIDTH); // mouse X offset from white key
+
+		if (offset < PW_BLACK_KEY_WIDTH / 2)
 		{
-			--key_num;
+			// At the location of a (possibly non-existent) black key on the left side
+			key -= neighboringBlackKeys.first;
 		}
-		if( key_num < NumKeys - 1 && Piano::isBlackKey( key_num+1 ) &&
-			offset >= ( PW_WHITE_KEY_WIDTH -
-					PW_BLACK_KEY_WIDTH / 2 ) )
+		else if (offset > PW_WHITE_KEY_WIDTH - (PW_BLACK_KEY_WIDTH / 2))
 		{
-			++key_num;
+			// At the location of a (possibly non-existent) black key on the right side
+			key += neighboringBlackKeys.second;
 		}
+
+		// For white keys in between black keys, no further adjustment is needed
 	}
 
-	// some range-checking-stuff
-	return qBound( 0, key_num, NumKeys - 1 );
+	return std::clamp(key, 0, NumKeys - 1);
 }
 
 
@@ -396,12 +391,12 @@ int PianoView::getKeyFromMouse( const QPoint & _p ) const
  *
  *  We need to update our start key position based on the new position.
  *
- *  \param _new_pos the new key position.
+ *  \param newPos the new key position, counting only white keys.
  */
-void PianoView::pianoScrolled(int new_pos)
+void PianoView::pianoScrolled(int newPos)
 {
-	m_startKey = static_cast<Octave>(new_pos / Piano::WhiteKeysPerOctave)
-		+ WhiteKeys[new_pos % Piano::WhiteKeysPerOctave];
+	m_startKey = static_cast<Octave>(newPos / Piano::WhiteKeysPerOctave)
+		+ WhiteKeys[newPos % Piano::WhiteKeysPerOctave];
 
 	update();
 }

src/gui/widgets/SimpleTextFloat.cpp

@@ -46,11 +46,11 @@ SimpleTextFloat::SimpleTextFloat() :
 	m_textLabel = new QLabel(this);
 	layout->addWidget(m_textLabel);
 
-	m_showTimer = new QTimer();
+	m_showTimer = new QTimer(this);
 	m_showTimer->setSingleShot(true);
 	QObject::connect(m_showTimer, &QTimer::timeout, this, &SimpleTextFloat::show);
 
-	m_hideTimer = new QTimer();
+	m_hideTimer = new QTimer(this);
 	m_hideTimer->setSingleShot(true);
 	QObject::connect(m_hideTimer, &QTimer::timeout, this, &SimpleTextFloat::hide);
 }