[PATCH] Splitter::ImplInitHorVer(bool) should always initialize all ...

[Mark Wielaard (via Code Review)]

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/4007

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/07/4007/1

Splitter::ImplInitHorVer(bool) should always initialize all state.

When creating a new Splitter ImplInitHorVer(bool) should always
init all state independent from the current state because some
variables might not be initialized yet.

 Conditional jump or move depends on uninitialised value(s)
    at 0x8293625: Splitter::ImplInitHorVer(bool) (split.cxx:70)
    by 0x82936DF: Splitter::ImplInit(Window*, long) (split.cxx:100)
    by 0x8293CC3: Splitter::Splitter(Window*, long) (split.cxx:157)
    by 0x2686BC48: ScTabSplitter::ScTabSplitter(Window*, long, ScViewData*) (tabsplit.cxx:28)
    ...
  Uninitialised value was created by a heap allocation
    at 0x4A08361: operator new(unsigned long) (vg_replace_malloc.c:298)
    by 0x2687D8F1: ScTabView::Init() (tabview5.cxx:90)
    by 0x2686C5D3: ScTabView::ScTabView(Window*, ScDocShell&, ScTabViewShell*) (tabview.cxx:261)
    ...

The public method to change the internal state is Splitter::SetHorizontal()
which does check the current state (and calls Splitter::ImplInitHorVer()
only when there is a real state change). So the bNew != mbHorzSplit was
redundant anyway.

Change-Id: If84e8bff4c87d208416de3583202ce88d6982a99
---
M vcl/source/window/split.cxx
1 file changed, 18 insertions(+), 18 deletions(-)



diff --git a/vcl/source/window/split.cxx b/vcl/source/window/split.cxx
index 92a637e..13cc486 100644
--- a/vcl/source/window/split.cxx
+++ b/vcl/source/window/split.cxx
@@ -65,28 +65,28 @@
 
 // -----------------------------------------------------------------------
 
+// Should only be called from a ImplInit method for initialization or
+// after checking bNew is different from the current mbHorzSplit value.
+// The public method that does that check is Splitter::SetHorizontal().
 void Splitter::ImplInitHorVer(bool bNew)
 {
-    if(bNew != (bool)mbHorzSplit)
+    mbHorzSplit = bNew;
+
+    PointerStyle ePointerStyle;
+    const StyleSettings& rSettings = GetSettings().GetStyleSettings();
+
+    if ( mbHorzSplit )
     {
-        mbHorzSplit = bNew;
-
-        PointerStyle ePointerStyle;
-        const StyleSettings& rSettings = GetSettings().GetStyleSettings();
-
-        if ( mbHorzSplit )
-        {
-            ePointerStyle = POINTER_HSPLIT;
-            SetSizePixel( Size( rSettings.GetSplitSize(), rSettings.GetScrollBarSize() ) );
-        }
-        else
-        {
-            ePointerStyle = POINTER_VSPLIT;
-            SetSizePixel( Size( rSettings.GetScrollBarSize(), rSettings.GetSplitSize() ) );
-        }
-
-        SetPointer( Pointer( ePointerStyle ) );
+        ePointerStyle = POINTER_HSPLIT;
+        SetSizePixel( Size( rSettings.GetSplitSize(), rSettings.GetScrollBarSize() ) );
     }
+    else
+    {
+        ePointerStyle = POINTER_VSPLIT;
+        SetSizePixel( Size( rSettings.GetScrollBarSize(), rSettings.GetSplitSize() ) );
+    }
+
+    SetPointer( Pointer( ePointerStyle ) );
 }
 
 // -----------------------------------------------------------------------

-- 
To view, visit https://gerrit.libreoffice.org/4007
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If84e8bff4c87d208416de3583202ce88d6982a99
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: Mark Wielaard <mark at klomp.org>