diff options
author | Alberto Mardegan <mardy@users.sourceforge.net> | 2013-05-24 14:39:37 +0300 |
---|---|---|
committer | Dirk Hohndel <dirk@hohndel.org> | 2013-05-24 06:18:27 -0700 |
commit | 5e0a3cdad8d02eff7ffa7c2abb4ac48c332f144f (patch) | |
tree | e6526ae3e1bc13da948133bd5e105604efa983e9 /qt-ui | |
parent | 99ecb4c8cb39f319c5de8ed326d4389ccbabf1fd (diff) | |
download | subsurface-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.cpp | 6 |
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); |