From 6d9c0146a5e1fcf481e99426921553cdbb0a336a Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Tue, 11 Aug 2020 17:43:25 +0200 Subject: [PATCH] BGradient: fix assignment operator, add copy constructor The BGradient class is a bit strange as it can store any gradient on its own, butonly the subclasses allow to set some of the fields. In the asignment operator, the non-base data (which is in an union) was not copied over. More importantly, the missing copy constructor led to the default implementation being used, and BList (used for the color stops) was being copied using its default copy constructor, resulting in the two BGradient (original and copy) poinitng to the same stops data. Heap corruption resulted whenever one of them was deleted. Having a working copy ocnstructor fixes this. The alternative is making the copy constructor private or protected to make sure gradients are not copied, since normally you'd copy only the subclasses, preserving the C++ type. However there is nothing enforcing that, and manipulating a BGradient copied from a subclass works just fine. Change-Id: I28e733eb8a2970b76ae623eabb75ef8435f508af Reviewed-on: https://review.haiku-os.org/c/haiku/+/3144 Reviewed-by: waddlesplash --- headers/os/interface/Gradient.h | 1 + src/kits/interface/Gradient.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/headers/os/interface/Gradient.h b/headers/os/interface/Gradient.h index 846141ac51..ca3f8035a9 100644 --- a/headers/os/interface/Gradient.h +++ b/headers/os/interface/Gradient.h @@ -49,6 +49,7 @@ public: public: BGradient(); + BGradient(const BGradient& other); BGradient(BMessage* archive); virtual ~BGradient(); diff --git a/src/kits/interface/Gradient.cpp b/src/kits/interface/Gradient.cpp index e4574e5960..0002e885ba 100644 --- a/src/kits/interface/Gradient.cpp +++ b/src/kits/interface/Gradient.cpp @@ -100,6 +100,14 @@ BGradient::BGradient() } +BGradient::BGradient(const BGradient& other) + : BArchivable(), + fColorStops(std::max((int32)4, other.CountColorStops())) +{ + *this = other; +} + + // constructor BGradient::BGradient(BMessage* archive) : BArchivable(archive), @@ -250,8 +258,30 @@ BGradient::Archive(BMessage* into, bool deep) const BGradient& BGradient::operator=(const BGradient& other) { + if (&other == this) + return *this; + SetColorStops(other); fType = other.fType; + switch (fType) { + case TYPE_LINEAR: + fData.linear = other.fData.linear; + break; + case TYPE_RADIAL: + fData.radial = other.fData.radial; + break; + case TYPE_RADIAL_FOCUS: + fData.radial_focus = other.fData.radial_focus; + break; + case TYPE_DIAMOND: + fData.diamond = other.fData.diamond; + break; + case TYPE_CONIC: + fData.conic = other.fData.conic; + break; + case TYPE_NONE: + break; + } return *this; }