summaryrefslogtreecommitdiffstats
path: root/qt-ui
diff options
context:
space:
mode:
authorGravatar Alberto Mardegan <mardy@users.sourceforge.net>2013-05-24 14:39:37 +0300
committerGravatar Dirk Hohndel <dirk@hohndel.org>2013-05-24 06:18:27 -0700
commit5e0a3cdad8d02eff7ffa7c2abb4ac48c332f144f (patch)
treee6526ae3e1bc13da948133bd5e105604efa983e9 /qt-ui
parent99ecb4c8cb39f319c5de8ed326d4389ccbabf1fd (diff)
downloadsubsurface-5e0a3cdad8d02eff7ffa7c2abb4ac48c332f144f.tar.gz
Fix usage of temporary QByteArrays
This commit fixes three different things: - a memory leak in WeightModel::setData() - getSetting() calling strdup() on a QByteArray - a possible usage of memory after deallocation Here's an explanation of the last issue (taken from the mailing list, slightly adapted): toByteArray(), as well as others "toSomething()" methods, returns a new object which the compiler allocates on the stack. The compiler will consider it a temporary data, and destroy it on the next line. So, when one does char *text= value.toByteArray().data(); // line 1 if (strcmp(description, text)) { // line 2 the compiler creates a QByteArray on line 1, calls ::data() on it, which returns a valid char *, and assigns its value to "text". So far, so good. But before jumping to line 2, the compiler destroys the temporary QByteArray, and this will in turn invoke the QByteArray destructor, which will destroy the internal data. The result is that on line 2, "text" will point to some memory which has already been freed. One solution is to store a copy of the temporary QByteArray into a local variable: the compiler will still destroy the temporary QByteArray it created, but (thanks to the reference-counted data sharing built in QByteArray) now the destructor will see that the data is referenced by another instance of QByteArray (the local variable "ba") and will not free the internal data. In this way, the internal data will be available until the local variable is destroyed, which will happen at the end of the {} block where it is defined. Please note that when one uses the data in the same line, one doesn't need to worry about this issue. In fact, text = strdup(value.toString().toUtf8().data()); works just fine. Signed-off-by: Alberto Mardegan <mardy@users.sourceforge.net> Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Diffstat (limited to 'qt-ui')
-rw-r--r--qt-ui/models.cpp6
1 files changed, 4 insertions, 2 deletions
diff --git a/qt-ui/models.cpp b/qt-ui/models.cpp
index 2bc21df42..6c6d0b133 100644
--- a/qt-ui/models.cpp
+++ b/qt-ui/models.cpp
@@ -139,7 +139,8 @@ bool CylindersModel::setData(const QModelIndex& index, const QVariant& value, in
switch(index.column()) {
case TYPE:
if (!value.isNull()) {
- char *text = value.toByteArray().data();
+ QByteArray ba = value.toByteArray();
+ const char *text = ba.constData();
if (!cyl->type.description || strcmp(cyl->type.description, text)) {
cyl->type.description = strdup(text);
mark_divelist_changed(TRUE);
@@ -373,7 +374,8 @@ bool WeightModel::setData(const QModelIndex& index, const QVariant& value, int r
switch(index.column()) {
case TYPE:
if (!value.isNull()) {
- char *text = strdup(value.toString().toUtf8().data());
+ QByteArray ba = value.toString().toUtf8();
+ const char *text = ba.constData();
if (!ws->description || strcmp(ws->description, text)) {
ws->description = strdup(text);
mark_divelist_changed(TRUE);