From 9c97217e3bedb2708bba6f23cbc6dd603569e077 Mon Sep 17 00:00:00 2001 From: Sami Rosendahl 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 --- 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(~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(~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