hashtags/lmms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 1 Wiki Activity

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

Browse Source 
* 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
This commit is contained in:
Dalton Messmer
2023-09-03 17:29:31 -04:00
committed by GitHub
parent e1d3ecb184
commit 0768f5ad2f
4 changed files with 64 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
plugins/LadspaEffect/caps/Descriptor.h
View File

@@ -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()

27
src/core/AutomationClip.cpp
View File

@@ -1106,16 +1106,16 @@ 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.
@@ -1123,14 +1123,6 @@ void AutomationClip::generateTangents(timeMap::iterator it, int numToGenerate)
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++;
}
}

107
src/gui/instrument/PianoView.cpp
View File

@@ -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 )
// 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;
// Calculate for full octaves
int key = octave * KeysPerOctave;
// 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)
{
if ( Piano::isBlackKey( m_startKey+i ) )
// 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;
}
}
for( int i = 0; i >= key_num; --i )
{
if ( Piano::isBlackKey( m_startKey+i ) )
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
}
key_num += m_startKey;
// is it a black key?
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 ) )
{
--key_num;
}
if( key_num < NumKeys - 1 && Piano::isBlackKey( key_num+1 ) &&
offset >= ( PW_WHITE_KEY_WIDTH -
PW_BLACK_KEY_WIDTH / 2 ) )
{
++key_num;
}
}
// 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();
}

4
src/gui/widgets/SimpleTextFloat.cpp
View File

@@ -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);
}