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 <sbergman@redhat.com>

1 files changed, 11 insertions, 11 deletions

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;