From 6222212a4ae0dbd5cedc081bbba4e90bcb32f068 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 4 Feb 2019 09:22:09 +0100 Subject: [PATCH 1/5] Fix double-free in WaveformMarkRange --- src/waveform/renderers/waveformmarkrange.cpp | 24 +++++--------------- src/waveform/renderers/waveformmarkrange.h | 10 ++++---- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/waveform/renderers/waveformmarkrange.cpp b/src/waveform/renderers/waveformmarkrange.cpp index 0d846de98dc..e787f8da1f8 100644 --- a/src/waveform/renderers/waveformmarkrange.cpp +++ b/src/waveform/renderers/waveformmarkrange.cpp @@ -8,18 +8,6 @@ #include "control/controlproxy.h" #include "widget/wskincolor.h" -WaveformMarkRange::WaveformMarkRange() - : m_markStartPointControl(NULL), - m_markEndPointControl(NULL), - m_markEnabledControl(NULL) { -} - -WaveformMarkRange::~WaveformMarkRange() { - delete m_markStartPointControl; - delete m_markEndPointControl; - delete m_markEnabledControl; -} - bool WaveformMarkRange::active() { const double startValue = start(); const double endValue = end(); @@ -71,18 +59,18 @@ void WaveformMarkRange::setup(const QString& group, const QDomNode& node, QString startControl = context.selectString(node, "StartControl"); if (!startControl.isEmpty()) { - m_markStartPointControl = - new ControlProxy(group, startControl); + DEBUG_ASSERT(!m_markStartPointControl); // has not been created yet + m_markStartPointControl = std::make_unique(group, startControl); } QString endControl = context.selectString(node, "EndControl"); if (!endControl.isEmpty()) { - m_markEndPointControl = - new ControlProxy(group, endControl); + DEBUG_ASSERT(!m_markEndPointControl); // has not been created yet + m_markEndPointControl = std::make_unique(group, endControl); } QString enabledControl = context.selectString(node, "EnabledControl"); if (!enabledControl.isEmpty()) { - m_markEnabledControl = - new ControlProxy(group, enabledControl); + DEBUG_ASSERT(!m_markEnabledControl); // has not been created yet + m_markEnabledControl = std::make_unique(group, enabledControl); } } diff --git a/src/waveform/renderers/waveformmarkrange.h b/src/waveform/renderers/waveformmarkrange.h index d8ea54c1c39..2d99978310f 100644 --- a/src/waveform/renderers/waveformmarkrange.h +++ b/src/waveform/renderers/waveformmarkrange.h @@ -4,6 +4,7 @@ #include #include "skin/skincontext.h" +#include "util/memory.h" class ControlProxy; class QDomNode; @@ -11,9 +12,6 @@ class WaveformSignalColors; class WaveformMarkRange { public: - WaveformMarkRange(); - ~WaveformMarkRange(); - // If a mark range is active it has valid start/end points so it should be // drawn on waveforms. bool active(); @@ -32,9 +30,9 @@ class WaveformMarkRange { private: void generateImage(int weidth, int height); - ControlProxy* m_markStartPointControl; - ControlProxy* m_markEndPointControl; - ControlProxy* m_markEnabledControl; + std::unique_ptr m_markStartPointControl; + std::unique_ptr m_markEndPointControl; + std::unique_ptr m_markEnabledControl; QColor m_activeColor; QColor m_disabledColor; From b0b063bf31d81e4ea849495301469767b48980ab Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 4 Feb 2019 15:22:02 +0100 Subject: [PATCH 2/5] Explicitly define/undefine move/copy constructors --- src/waveform/renderers/waveformmarkrange.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/waveform/renderers/waveformmarkrange.h b/src/waveform/renderers/waveformmarkrange.h index 2d99978310f..fe640755618 100644 --- a/src/waveform/renderers/waveformmarkrange.h +++ b/src/waveform/renderers/waveformmarkrange.h @@ -12,6 +12,11 @@ class WaveformSignalColors; class WaveformMarkRange { public: + WaveformMarkRange() = default; + // This class is only moveable, but not copyable! + WaveformMarkRange(WaveformMarkRange&&) = default; + WaveformMarkRange(const WaveformMarkRange&) = delete; + // If a mark range is active it has valid start/end points so it should be // drawn on waveforms. bool active(); From cf3bd22f343b2d041b5cdc9360d5f38f5f9c207d Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 4 Feb 2019 21:03:59 +0100 Subject: [PATCH 3/5] Transform member function into constructor --- src/waveform/renderers/waveformmarkrange.cpp | 12 +++++++----- src/waveform/renderers/waveformmarkrange.h | 8 +++----- src/waveform/renderers/waveformrendermarkrange.cpp | 12 ++++++------ src/waveform/renderers/waveformrendermarkrange.h | 9 +++------ src/widget/woverview.cpp | 3 +-- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/waveform/renderers/waveformmarkrange.cpp b/src/waveform/renderers/waveformmarkrange.cpp index e787f8da1f8..0dc5a2d6e16 100644 --- a/src/waveform/renderers/waveformmarkrange.cpp +++ b/src/waveform/renderers/waveformmarkrange.cpp @@ -36,10 +36,13 @@ double WaveformMarkRange::end() { return end; } -void WaveformMarkRange::setup(const QString& group, const QDomNode& node, - const SkinContext& context, - const WaveformSignalColors& signalColors) { - m_activeColor = context.selectString(node, "Color"); +WaveformMarkRange::WaveformMarkRange( + const QString& group, + const QDomNode& node, + const SkinContext& context, + const WaveformSignalColors& signalColors) + : m_activeColor(context.selectString(node, "Color")), + m_disabledColor(context.selectString(node, "DisabledColor")) { if (!m_activeColor.isValid()) { //vRince kind of legacy fallback ... // As a fallback, grab the mark color from the parent's MarkerColor @@ -49,7 +52,6 @@ void WaveformMarkRange::setup(const QString& group, const QDomNode& node, m_activeColor = WSkinColor::getCorrectColor(m_activeColor); } - m_disabledColor = context.selectString(node, "DisabledColor"); if (!m_disabledColor.isValid()) { //vRince kind of legacy fallback ... // Read the text color, otherwise use the parent's SignalColor. diff --git a/src/waveform/renderers/waveformmarkrange.h b/src/waveform/renderers/waveformmarkrange.h index fe640755618..9657a86b264 100644 --- a/src/waveform/renderers/waveformmarkrange.h +++ b/src/waveform/renderers/waveformmarkrange.h @@ -12,7 +12,9 @@ class WaveformSignalColors; class WaveformMarkRange { public: - WaveformMarkRange() = default; + WaveformMarkRange(const QString &group, const QDomNode& node, + const SkinContext& context, + const WaveformSignalColors& signalColors); // This class is only moveable, but not copyable! WaveformMarkRange(WaveformMarkRange&&) = default; WaveformMarkRange(const WaveformMarkRange&) = delete; @@ -28,10 +30,6 @@ class WaveformMarkRange { // Returns end value or -1 if the end control doesn't exist. double end(); - void setup(const QString &group, const QDomNode& node, - const SkinContext& context, - const WaveformSignalColors& signalColors); - private: void generateImage(int weidth, int height); diff --git a/src/waveform/renderers/waveformrendermarkrange.cpp b/src/waveform/renderers/waveformrendermarkrange.cpp index 00e82b5a649..ab0d8ec7870 100644 --- a/src/waveform/renderers/waveformrendermarkrange.cpp +++ b/src/waveform/renderers/waveformrendermarkrange.cpp @@ -18,9 +18,6 @@ WaveformRenderMarkRange::WaveformRenderMarkRange(WaveformWidgetRenderer* wavefor WaveformRendererAbstract(waveformWidgetRenderer) { } -WaveformRenderMarkRange::~WaveformRenderMarkRange() { -} - void WaveformRenderMarkRange::setup(const QDomNode& node, const SkinContext& context) { m_markRanges.clear(); m_markRanges.reserve(1); @@ -28,9 +25,12 @@ void WaveformRenderMarkRange::setup(const QDomNode& node, const SkinContext& con QDomNode child = node.firstChild(); while (!child.isNull()) { if (child.nodeName() == "MarkRange") { - m_markRanges.push_back(WaveformMarkRange()); - m_markRanges.back().setup(m_waveformRenderer->getGroup(), child, - context, *m_waveformRenderer->getWaveformSignalColors()); + m_markRanges.push_back( + WaveformMarkRange( + m_waveformRenderer->getGroup(), + child, + context, + *m_waveformRenderer->getWaveformSignalColors())); } child = child.nextSibling(); } diff --git a/src/waveform/renderers/waveformrendermarkrange.h b/src/waveform/renderers/waveformrendermarkrange.h index a3655fcb1ed..7307a339958 100644 --- a/src/waveform/renderers/waveformrendermarkrange.h +++ b/src/waveform/renderers/waveformrendermarkrange.h @@ -11,7 +11,6 @@ #include "preferences/usersettings.h" #include "skin/skincontext.h" -#include "util/class.h" #include "waveform/renderers/waveformmarkrange.h" #include "waveform/renderers/waveformrendererabstract.h" @@ -21,17 +20,15 @@ class ControlObject; class WaveformRenderMarkRange : public WaveformRendererAbstract { public: explicit WaveformRenderMarkRange(WaveformWidgetRenderer* waveformWidgetRenderer); - virtual ~WaveformRenderMarkRange(); + ~WaveformRenderMarkRange() override = default; - virtual void setup(const QDomNode& node, const SkinContext& context); - virtual void draw(QPainter* painter, QPaintEvent* event); + void setup(const QDomNode& node, const SkinContext& context) override; + void draw(QPainter* painter, QPaintEvent* event) override; private: void generateImages(); std::vector m_markRanges; - - DISALLOW_COPY_AND_ASSIGN(WaveformRenderMarkRange); }; #endif diff --git a/src/widget/woverview.cpp b/src/widget/woverview.cpp index 9d14821c438..fc38a91d45e 100644 --- a/src/widget/woverview.cpp +++ b/src/widget/woverview.cpp @@ -96,9 +96,8 @@ void WOverview::setup(const QDomNode& node, const SkinContext& context) { QDomNode child = node.firstChild(); while (!child.isNull()) { if (child.nodeName() == "MarkRange") { - m_markRanges.push_back(WaveformMarkRange()); + m_markRanges.push_back(WaveformMarkRange(m_group, child, context, m_signalColors)); WaveformMarkRange& markRange = m_markRanges.back(); - markRange.setup(m_group, child, context, m_signalColors); if (markRange.m_markEnabledControl) { markRange.m_markEnabledControl->connectValueChanged( From 5af7b03f59df161b6e557eafa5243f6a1bc4470d Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 4 Feb 2019 21:08:33 +0100 Subject: [PATCH 4/5] Add missing const qualifiers --- src/waveform/renderers/waveformmarkrange.cpp | 8 ++++---- src/waveform/renderers/waveformmarkrange.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/waveform/renderers/waveformmarkrange.cpp b/src/waveform/renderers/waveformmarkrange.cpp index 0dc5a2d6e16..615ab120877 100644 --- a/src/waveform/renderers/waveformmarkrange.cpp +++ b/src/waveform/renderers/waveformmarkrange.cpp @@ -8,19 +8,19 @@ #include "control/controlproxy.h" #include "widget/wskincolor.h" -bool WaveformMarkRange::active() { +bool WaveformMarkRange::active() const { const double startValue = start(); const double endValue = end(); return startValue != endValue && startValue != -1.0 && endValue != -1.0; } -bool WaveformMarkRange::enabled() { +bool WaveformMarkRange::enabled() const { // Default to enabled if there is no enabled control. return !m_markEnabledControl || !m_markEnabledControl->valid() || m_markEnabledControl->get() > 0.0; } -double WaveformMarkRange::start() { +double WaveformMarkRange::start() const { double start = -1.0; if (m_markStartPointControl && m_markStartPointControl->valid()) { start = m_markStartPointControl->get(); @@ -28,7 +28,7 @@ double WaveformMarkRange::start() { return start; } -double WaveformMarkRange::end() { +double WaveformMarkRange::end() const { double end = -1.0; if (m_markEndPointControl && m_markEndPointControl->valid()) { end = m_markEndPointControl->get(); diff --git a/src/waveform/renderers/waveformmarkrange.h b/src/waveform/renderers/waveformmarkrange.h index 9657a86b264..501cce871a4 100644 --- a/src/waveform/renderers/waveformmarkrange.h +++ b/src/waveform/renderers/waveformmarkrange.h @@ -21,14 +21,14 @@ class WaveformMarkRange { // If a mark range is active it has valid start/end points so it should be // drawn on waveforms. - bool active(); + bool active() const; // If a mark range is enabled that means it should be painted with its // active color instead of its disabled color. - bool enabled(); + bool enabled() const; // Returns start value or -1 if the start control doesn't exist. - double start(); + double start() const; // Returns end value or -1 if the end control doesn't exist. - double end(); + double end() const; private: void generateImage(int weidth, int height); From f5203ae834ce33afead02c63280d9a41d3a93a29 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Mon, 4 Feb 2019 23:40:44 +0100 Subject: [PATCH 5/5] Fix various code style issues --- src/waveform/renderers/waveformmarkrange.cpp | 59 ++++++++++---------- src/waveform/renderers/waveformmarkrange.h | 19 ++++--- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/waveform/renderers/waveformmarkrange.cpp b/src/waveform/renderers/waveformmarkrange.cpp index 615ab120877..6cc50adc07d 100644 --- a/src/waveform/renderers/waveformmarkrange.cpp +++ b/src/waveform/renderers/waveformmarkrange.cpp @@ -4,38 +4,9 @@ #include "waveformmarkrange.h" #include "waveformsignalcolors.h" -#include "control/controlobject.h" -#include "control/controlproxy.h" +#include "skin/skincontext.h" #include "widget/wskincolor.h" -bool WaveformMarkRange::active() const { - const double startValue = start(); - const double endValue = end(); - return startValue != endValue && startValue != -1.0 && endValue != -1.0; -} - -bool WaveformMarkRange::enabled() const { - // Default to enabled if there is no enabled control. - return !m_markEnabledControl || !m_markEnabledControl->valid() || - m_markEnabledControl->get() > 0.0; -} - -double WaveformMarkRange::start() const { - double start = -1.0; - if (m_markStartPointControl && m_markStartPointControl->valid()) { - start = m_markStartPointControl->get(); - } - return start; -} - -double WaveformMarkRange::end() const { - double end = -1.0; - if (m_markEndPointControl && m_markEndPointControl->valid()) { - end = m_markEndPointControl->get(); - } - return end; -} - WaveformMarkRange::WaveformMarkRange( const QString& group, const QDomNode& node, @@ -76,6 +47,34 @@ WaveformMarkRange::WaveformMarkRange( } } +bool WaveformMarkRange::active() const { + const double startValue = start(); + const double endValue = end(); + return startValue != endValue && startValue != -1.0 && endValue != -1.0; +} + +bool WaveformMarkRange::enabled() const { + // Default to enabled if there is no enabled control. + return !m_markEnabledControl || !m_markEnabledControl->valid() || + m_markEnabledControl->get() > 0.0; +} + +double WaveformMarkRange::start() const { + double start = -1.0; + if (m_markStartPointControl && m_markStartPointControl->valid()) { + start = m_markStartPointControl->get(); + } + return start; +} + +double WaveformMarkRange::end() const { + double end = -1.0; + if (m_markEndPointControl && m_markEndPointControl->valid()) { + end = m_markEndPointControl->get(); + } + return end; +} + void WaveformMarkRange::generateImage(int weidth, int height) { m_activeImage = QImage(weidth, height, QImage::Format_ARGB32_Premultiplied); m_disabledImage = QImage(weidth, height, QImage::Format_ARGB32_Premultiplied); diff --git a/src/waveform/renderers/waveformmarkrange.h b/src/waveform/renderers/waveformmarkrange.h index 501cce871a4..eb3d0a93530 100644 --- a/src/waveform/renderers/waveformmarkrange.h +++ b/src/waveform/renderers/waveformmarkrange.h @@ -1,20 +1,23 @@ -#ifndef WAVEFORMMARKRANGE_H -#define WAVEFORMMARKRANGE_H +#pragma once +#include #include +#include -#include "skin/skincontext.h" +#include "control/controlproxy.h" #include "util/memory.h" -class ControlProxy; class QDomNode; +class SkinContext; class WaveformSignalColors; class WaveformMarkRange { public: - WaveformMarkRange(const QString &group, const QDomNode& node, - const SkinContext& context, - const WaveformSignalColors& signalColors); + WaveformMarkRange( + const QString& group, + const QDomNode& node, + const SkinContext& context, + const WaveformSignalColors& signalColors); // This class is only moveable, but not copyable! WaveformMarkRange(WaveformMarkRange&&) = default; WaveformMarkRange(const WaveformMarkRange&) = delete; @@ -46,5 +49,3 @@ class WaveformMarkRange { friend class WaveformRenderMarkRange; friend class WOverview; }; - -#endif // WAVEFORMMARKRANGE_H