vcl/inc/unx/saldisp.hxx | 24 +++++++++++++++++++++++-
vcl/unx/generic/app/saldisp.cxx | 22 +++++++++++-----------
2 files changed, 34 insertions(+), 12 deletions(-)
New commits:
commit a1f5b1f30a5a80d7eeed2efe2564763bf1d12086
Author: Stephan Bergmann <sberg...@redhat.com>
Date: Thu Jun 28 09:47:29 2018 +0200
Clean up ownership tracking of SalVisual's visual (in X11 Visual base class)
GCC trunk towards GCC 9 emits -Wdeprecated-copy because SalVisual has
implicit
copy functions but a user-declared dtor. The implicitly-defined copy
functions
are actually used (so cannot be deleted), but in fragile interaction with
the
semantics of the user-provided dtor in the SalColormap(sal_uInt16) ctor.
Changing SalVisual into a move-only class (moving the "this instance must
delete
its visual member" information) doesn't work well, as
SalDisplay::ScreenData is
used as a supply of SalVisual instances (but which will never be of the
"this
instance must delete its visual member" variety) that are then copied into
SalColormap::m_aVisual.
As only SalColormap creates SalVisual instances of the "this instance must
delete its visual member" variety, what actually works is to track that
information in SalColormap instead of directly in SalVisual, and make
SalColormap a move-only class.
Change-Id: Ib968a38c40b660ce91981b3c7b281f4add6ece6b
Reviewed-on: https://gerrit.libreoffice.org/56579
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/vcl/inc/unx/saldisp.hxx b/vcl/inc/unx/saldisp.hxx
index a53be3692d8b..0bef7afb6c52 100644
--- a/vcl/inc/unx/saldisp.hxx
+++ b/vcl/inc/unx/saldisp.hxx
@@ -38,6 +38,7 @@ class SalXLib;
#include <vcl/ptrstyle.hxx>
#include <sal/types.h>
#include <osl/mutex.h>
+#include <cassert>
#include <list>
#include <unordered_map>
#include <vector>
@@ -89,7 +90,6 @@ class SalVisual : public XVisualInfo
int nBlueBits_;
public:
SalVisual();
- ~SalVisual();
SalVisual( const XVisualInfo* pXVI );
VisualID GetVisualId() const { return visualid; }
@@ -101,12 +101,29 @@ public:
Color GetTCColor( Pixel nPixel ) const;
};
+// A move-only flag, used by SalColormap to track ownership of its
m_aVisual.visual:
+struct OwnershipFlag {
+ bool owner = false;
+
+ OwnershipFlag() = default;
+
+ OwnershipFlag(OwnershipFlag && other) noexcept: owner(other.owner) {
other.owner = false; }
+
+ OwnershipFlag & operator =(OwnershipFlag && other) noexcept {
+ assert(&other != this);
+ owner = other.owner;
+ other.owner = false;
+ return *this;
+ }
+};
+
class SalColormap
{
const SalDisplay* m_pDisplay;
Colormap m_hColormap;
std::vector<Color> m_aPalette; // Pseudocolor
SalVisual m_aVisual;
+ OwnershipFlag m_aVisualOwnership;
std::vector<sal_uInt16> m_aLookupTable; // Pseudocolor: 12bit
reduction
Pixel m_nWhitePixel;
Pixel m_nBlackPixel;
@@ -122,6 +139,11 @@ public:
SalColormap( sal_uInt16 nDepth );
SalColormap();
+ ~SalColormap();
+
+ SalColormap(SalColormap &&) = default;
+ SalColormap & operator =(SalColormap &&) = default;
+
Colormap GetXColormap() const { return m_hColormap; }
const SalDisplay* GetDisplay() const { return m_pDisplay; }
inline Display* GetXDisplay() const;
diff --git a/vcl/unx/generic/app/saldisp.cxx b/vcl/unx/generic/app/saldisp.cxx
index 5d266828196a..dff49c8e8b7c 100644
--- a/vcl/unx/generic/app/saldisp.cxx
+++ b/vcl/unx/generic/app/saldisp.cxx
@@ -2435,11 +2435,6 @@ SalVisual::SalVisual( const XVisualInfo* pXVI )
}
}
-SalVisual::~SalVisual()
-{
- if( -1 == screen && VisualID(-1) == visualid ) delete visual;
-}
-
// Converts the order of bytes of a Pixel into bytes of a Color
// This is not reversible for the 6 XXXA
@@ -2608,8 +2603,8 @@ SalColormap::SalColormap( sal_uInt16 nDepth )
&aVI ) )
{
aVI.visual = new Visual;
- aVI.visualid = VisualID(0); // beware of temporary
destructor below
- aVI.screen = 0;
+ aVI.visualid = VisualID(-1);
+ aVI.screen = -1;
aVI.depth = nDepth;
aVI.c_class = TrueColor;
if( 24 == nDepth ) // 888
@@ -2661,16 +2656,21 @@ SalColormap::SalColormap( sal_uInt16 nDepth )
aVI.visual->map_entries = aVI.colormap_size;
m_aVisual = SalVisual( &aVI );
- // give ownership of constructed Visual() to m_aVisual
- // see SalVisual destructor
- m_aVisual.visualid = VisualID(-1);
- m_aVisual.screen = -1;
+ m_aVisualOwnership.owner = true;
}
else
m_aVisual = SalVisual( &aVI );
}
}
+SalColormap::~SalColormap()
+{
+ if (m_aVisualOwnership.owner)
+ {
+ delete m_aVisual.visual;
+ }
+}
+
void SalColormap::GetPalette()
{
Pixel i;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
