From 0822ec27a9fde7fac91a39214286c5d0d0eeb154 Mon Sep 17 00:00:00 2001 From: PulkoMandy Date: Tue, 17 Oct 2023 16:04:51 +0200 Subject: [PATCH] libicon: Fix dangling references to VertexSource in PathTransformer PathTransformer keeps a reference to the VertexSource used (which can be either another PathTransformer, or the base path for a shape). When a shape is cloned, this should be retargetted to the new object in the clone. However, references are not settable, and so, what would happen instead is the original (non-cloned) vertex source was reinitialized (through the default implementation of the assignment operator) using data from the clone, and the clone would still point to it. Then, if the original object is deleted, the clone would point to freed memory. To avoid this problem, replace the reference with a pointer (which can be set to point elsewhere, as the code intended to do). To make sure this does not happen again, make the VertexSource copy constructor and assignment operator private, and deleted when the compiler supports that. Fixes #18577 Change-Id: I8870d9471c5064e922a84eff8447cbda783b13e6 Reviewed-on: https://review.haiku-os.org/c/haiku/+/7052 Reviewed-by: Adrien Destugues Tested-by: Commit checker robot Reviewed-by: Zardshard <0azrune6@zard.anonaddy.com> --- src/libs/icon/generic/VertexSource.h | 11 +++++++++++ src/libs/icon/transformer/AffineTransformer.cpp | 4 ++-- src/libs/icon/transformer/ContourTransformer.cpp | 4 ++-- src/libs/icon/transformer/PathTransformer.h | 14 +++++++------- .../icon/transformer/PerspectiveTransformer.cpp | 8 ++++---- src/libs/icon/transformer/StrokeTransformer.cpp | 4 ++-- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/libs/icon/generic/VertexSource.h b/src/libs/icon/generic/VertexSource.h index 3c1c9d0ded..7eda69f81f 100644 --- a/src/libs/icon/generic/VertexSource.h +++ b/src/libs/icon/generic/VertexSource.h @@ -27,6 +27,17 @@ class VertexSource { /*! Determines whether open paths should be closed or left open. */ virtual bool WantsOpenPaths() const = 0; virtual double ApproximationScale() const = 0; + + private: + // Not allowed +#if __GNUC__ <= 2 +#define NOT_ALLOWED +#else +#define NOT_ALLOWED = delete +#endif + VertexSource(const VertexSource& other) NOT_ALLOWED; + VertexSource& operator=(const VertexSource& other) NOT_ALLOWED; +#undef NOT_ALLOWED }; diff --git a/src/libs/icon/transformer/AffineTransformer.cpp b/src/libs/icon/transformer/AffineTransformer.cpp index 2d7d3fb088..b0b32ac523 100644 --- a/src/libs/icon/transformer/AffineTransformer.cpp +++ b/src/libs/icon/transformer/AffineTransformer.cpp @@ -61,7 +61,7 @@ AffineTransformer::~AffineTransformer() Transformer* AffineTransformer::Clone() const { - AffineTransformer* clone = new (nothrow) AffineTransformer(fSource); + AffineTransformer* clone = new (nothrow) AffineTransformer(*fSource); if (clone) clone->multiply(*this); return clone; @@ -93,7 +93,7 @@ AffineTransformer::SetSource(VertexSource& source) double AffineTransformer::ApproximationScale() const { - return fabs(fSource.ApproximationScale() * scale()); + return fabs(fSource->ApproximationScale() * scale()); } // #pragma mark - diff --git a/src/libs/icon/transformer/ContourTransformer.cpp b/src/libs/icon/transformer/ContourTransformer.cpp index a569513f45..7fd32081d4 100644 --- a/src/libs/icon/transformer/ContourTransformer.cpp +++ b/src/libs/icon/transformer/ContourTransformer.cpp @@ -73,7 +73,7 @@ ContourTransformer::~ContourTransformer() Transformer* ContourTransformer::Clone() const { - ContourTransformer* clone = new (nothrow) ContourTransformer(fSource); + ContourTransformer* clone = new (nothrow) ContourTransformer(*fSource); if (clone) { clone->line_join(line_join()); clone->inner_join(inner_join()); @@ -111,7 +111,7 @@ ContourTransformer::SetSource(VertexSource& source) double ContourTransformer::ApproximationScale() const { - double scale = fSource.ApproximationScale(); + double scale = fSource->ApproximationScale(); double factor = fabs(width()); if (factor > 1.0) scale *= factor; diff --git a/src/libs/icon/transformer/PathTransformer.h b/src/libs/icon/transformer/PathTransformer.h index eb75be76e9..0be9e02737 100644 --- a/src/libs/icon/transformer/PathTransformer.h +++ b/src/libs/icon/transformer/PathTransformer.h @@ -24,25 +24,25 @@ class PathTransformer : public VertexSource { public: PathTransformer(VertexSource& source) - : fSource(source) {} + : fSource(&source) {} virtual ~PathTransformer() {} // PathTransformer virtual void rewind(unsigned path_id) - { fSource.rewind(path_id); } + { fSource->rewind(path_id); } virtual unsigned vertex(double* x, double* y) - { return fSource.vertex(x, y); } + { return fSource->vertex(x, y); } virtual void SetSource(VertexSource& source) - { fSource = source; } + { fSource = &source; } virtual bool WantsOpenPaths() const - { return fSource.WantsOpenPaths(); } + { return fSource->WantsOpenPaths(); } virtual double ApproximationScale() const - { return fSource.ApproximationScale(); } + { return fSource->ApproximationScale(); } protected: - VertexSource& fSource; + VertexSource* fSource; }; diff --git a/src/libs/icon/transformer/PerspectiveTransformer.cpp b/src/libs/icon/transformer/PerspectiveTransformer.cpp index f4659c846f..60ba8328d2 100644 --- a/src/libs/icon/transformer/PerspectiveTransformer.cpp +++ b/src/libs/icon/transformer/PerspectiveTransformer.cpp @@ -76,8 +76,8 @@ PerspectiveTransformer::PerspectiveTransformer(const PerspectiveTransformer& oth #else : Transformer(""), #endif - PathTransformer(other.fSource), - Perspective(fSource, *this), + PathTransformer(*other.fSource), + Perspective(*fSource, *this), fShape(other.fShape) #ifdef ICON_O_MATIC , fInverted(other.fInverted), @@ -162,7 +162,7 @@ PerspectiveTransformer::SetSource(VertexSource& source) double PerspectiveTransformer::ApproximationScale() const { - return fSource.ApproximationScale() * scale(); + return fSource->ApproximationScale() * scale(); } @@ -223,7 +223,7 @@ PerspectiveTransformer::ObjectChanged(const Observable* object) uint32 pathID[1]; pathID[0] = 0; double left, top, right, bottom; - agg::bounding_rect(fSource, pathID, 0, 1, &left, &top, &right, &bottom); + agg::bounding_rect(*fSource, pathID, 0, 1, &left, &top, &right, &bottom); BRect newFromBox = BRect(left, top, right, bottom); // Stop if nothing we care about has changed diff --git a/src/libs/icon/transformer/StrokeTransformer.cpp b/src/libs/icon/transformer/StrokeTransformer.cpp index 4ef926ea24..0b03098569 100644 --- a/src/libs/icon/transformer/StrokeTransformer.cpp +++ b/src/libs/icon/transformer/StrokeTransformer.cpp @@ -76,7 +76,7 @@ StrokeTransformer::~StrokeTransformer() Transformer* StrokeTransformer::Clone() const { - StrokeTransformer* clone = new (nothrow) StrokeTransformer(fSource); + StrokeTransformer* clone = new (nothrow) StrokeTransformer(*fSource); if (clone) { clone->line_cap(line_cap()); clone->line_join(line_join()); @@ -122,7 +122,7 @@ StrokeTransformer::WantsOpenPaths() const double StrokeTransformer::ApproximationScale() const { - double scale = fSource.ApproximationScale(); + double scale = fSource->ApproximationScale(); double factor = fabs(width()); if (factor > 1.0) scale *= factor;