From 9c97217e3bedb2708bba6f23cbc6dd603569e077 Mon Sep 17 00:00:00 2001
From: Sami Rosendahl <ext-sami.1.rosendahl@nokia.com>
Date: Mon, 6 Feb 2012 10:00:10 +0200
Subject: Fix access to uninitialized values in QtXmlPatterns

Fixes valgrind warning like below when executing tst_QXmlQuery::copyConstructor()
  Conditional jump or move depends on uninitialised value(s)
    at: QPatternist::NodeIndexStorage::operator!=(QPatternist::NodeIndexStorage const&) const (q
    by: QXmlItem::operator=(QXmlItem const&) (qabstractxmlnodemodel.cpp:1228)

Reason for the warning is that QPatternist::NodeIndexStorage::operator!=
accesses all fields of NodeIndexStorage, which are all not intialized in
every execution path of QXmlItem::QXmlItem(const QVariant &) and class
QPatternist::Item constructors.

Fixed by adding NodeIndexStorage::reset() function that resets all fields
and put a call to that function where NodeIndexStorage objects were
previously incompletely initialized. Note that unfortunately class
NodeIndexStorage cannot have a default constructor, because it is used as
a union field.

Change-Id: I686433ba552f025658f7e583226e77346db82159
(cherry picked from commit f42f82f435d738339ad85c1380d1167338517247)
(cherry picked from commit 65d2458408ccda1b37e1069fd13791a60fa0c672)
Reviewed-by: Robin Burchell <robin+qt@viroteck.net>
---
 src/xmlpatterns/api/qabstractxmlnodemodel.cpp | 11 ++---------
 src/xmlpatterns/api/qabstractxmlnodemodel.h   | 11 ++++++++---
 src/xmlpatterns/data/qitem_p.h                | 21 ++++-----------------
 3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/src/xmlpatterns/api/qabstractxmlnodemodel.cpp b/src/xmlpatterns/api/qabstractxmlnodemodel.cpp
index 81c6473..e0a04f7 100644
--- a/src/xmlpatterns/api/qabstractxmlnodemodel.cpp
+++ b/src/xmlpatterns/api/qabstractxmlnodemodel.cpp
@@ -1138,9 +1138,7 @@ bool QAbstractXmlNodeModel::isDeepEqual(const QXmlNodeModelIndex &n1,
  */
 QXmlItem::QXmlItem()
 {
-    m_node.model = 0;
-    m_node.data = 0;
-    m_node.additionalData = 0;
+    m_node.reset();
 }
 
 bool QXmlItem::internalIsAtomicValue() const
@@ -1164,12 +1162,10 @@ QXmlItem::QXmlItem(const QXmlItem &other) : m_node(other.m_node)
  */
 QXmlItem::QXmlItem(const QVariant &atomicValue)
 {
+    m_node.reset();
     if(atomicValue.isNull())
     {
         /* Then we behave just like the default constructor. */
-        m_node.model = 0;
-        m_node.data = 0;
-        m_node.additionalData = 0;
         return;
     }
 
@@ -1188,10 +1184,7 @@ QXmlItem::QXmlItem(const QVariant &atomicValue)
     else
     {
         m_atomicValue = 0;
-        m_node.model = 0;
     }
-
-    m_node.additionalData = 0;
 }
 
 /*!
diff --git a/src/xmlpatterns/api/qabstractxmlnodemodel.h b/src/xmlpatterns/api/qabstractxmlnodemodel.h
index f8663ed..8c2f7ec 100644
--- a/src/xmlpatterns/api/qabstractxmlnodemodel.h
+++ b/src/xmlpatterns/api/qabstractxmlnodemodel.h
@@ -105,6 +105,13 @@ namespace QPatternist
 
         /* Implementation is in qabstractxmlnodemodel.cpp. */
         inline bool operator!=(const NodeIndexStorage &other) const;
+
+        void reset()
+        {
+            data = 0;
+            additionalData = 0;
+            model = 0;
+        }
     };
 }
 
@@ -217,9 +224,7 @@ public:
 
     inline void reset()
     {
-        m_storage.data = 0;
-        m_storage.additionalData = 0;
-        m_storage.model = 0;
+        m_storage.reset();
     }
 
 private:
diff --git a/src/xmlpatterns/data/qitem_p.h b/src/xmlpatterns/data/qitem_p.h
index 33cf9a0..e037aec 100644
--- a/src/xmlpatterns/data/qitem_p.h
+++ b/src/xmlpatterns/data/qitem_p.h
@@ -207,14 +207,7 @@ namespace QPatternist
          */
         inline Item()
         {
-            /* Note that this function should be equal to reset(). */
-
-            /* This is the area which atomicValue uses. Becauase we want as()
-             * to return null on null-constructed objects, we initialize it. */
-            node.data = 0;
-
-            /* This signals that we're not an atomic value. */
-            node.model = 0;
+            node.reset();
         }
 
         inline Item(const QXmlNodeModelIndex &n) : node(n.m_storage)
@@ -231,6 +224,7 @@ namespace QPatternist
 
         inline Item(const AtomicValue::Ptr &a)
         {
+            node.reset();
             if(a)
             {
                 atomicValue = a.data();
@@ -239,14 +233,12 @@ namespace QPatternist
                 /* Signal that we're housing an atomic value. */
                 node.model = reinterpret_cast<const QAbstractXmlNodeModel *>(~0);
             }
-            else
-                node.model = 0; /* Like the default constructor. */
         }
 
         inline Item(const AtomicValue *const a)
         {
             /* Note, the implementation is a copy of the constructor above. */
-
+            node.reset();
             if(a)
             {
                 atomicValue = a;
@@ -255,8 +247,6 @@ namespace QPatternist
                 /* Signal that we're housing an atomic value. */
                 node.model = reinterpret_cast<const QAbstractXmlNodeModel *>(~0);
             }
-            else
-                node.model = 0; /* Like the default constructor. */
         }
 
         inline ~Item()
@@ -412,10 +402,7 @@ namespace QPatternist
             if(isAtomicValue() && !atomicValue->ref.deref())
                 delete atomicValue;
 
-            /* Note that this function should be equal to the default
-             * constructor. */
-            node.model = 0;
-            node.data = 0;
+            node.reset();
         }
 
         static inline Item fromPublic(const QXmlItem &i)
-- 
cgit v0.12