mifr/subsurface
1
0
Fork 0
mirror of https://github.com/subsurface/subsurface.git synced 2025-01-31 21:53:23 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix usage of temporary QByteArrays

Browse source 
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>
This commit is contained in:
Alberto Mardegan 2013-05-24 14:39:37 +03:00 • committed by Dirk Hohndel
parent 99ecb4c8cb
commit 5e0a3cdad8
2 changed files with 5 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
qt-gui.cpp
View file

@ -74,11 +74,9 @@ void init_qt_ui(int *argcp, char ***argvp, char *errormessage)
const char *getSetting(QSettings &s, QString name)
{
QVariant v;
QString text;
v = s.value(name);
if (v.isValid()) {
text = v.toString();
return strdup(text.toUtf8());
return strdup(v.toString().toUtf8().constData());
}
return NULL;
}

6
qt-ui/models.cpp
View file

@ -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);