Opened 18 years ago

Closed 18 years ago

Last modified 15 years ago

#17 closed bug (fixed)

Returning address of local variable

Reported by: mloskot Owned by: g_j_m
Priority: critical: causes crash or data corruption Milestone:
Component: Vectors Version: Trunk
Keywords: Cc:
Must Fix for Release: No Platform: All
Platform Version: Awaiting user input: no

Description (last modified by gsherman)

In file qgsfeature.cpp, member function: QString const& !QgsFeature::wellKnownText() const returns const reference to QString but in the body of this function QString::null is returned. null is an object of type QString::Null, so this return expression returns reference to local (automatic) variable. After program execution flow exits this function, this reference is invalid and undefined behaviour can be expected.

Here is small example that presents how easily the problem explained above can cause serious crash:

#include <QCoreApplication>
#include <QString>
#include <iostream>

struct B
{
   QString const& foo()
   {
      return QString::null;
   }
};

int main(int argc, char *argv[])
{
   QCoreApplication app(argc, argv);
   B b;
   std::cout << b.foo().toStdString() << std::endl;
   //------------^^^^^^^^^^^^^^^^^^^ ka-Boom!
   return 0;
}

here, toStdString() is called on invalid reference to not existing object, so program will crash with "Access violation" error.

IMPORTANT''' I'd recommend to make a detailed review of *all* places where const reference to QString is returned from function or is passed to function and QString::null is used as a default argument value.

This example above presents only return value case but following example may also cause problems:

void foo(QString const& s = QString::null)
{
   // do something with s without checking
   // if its value is QString::null
   std::cout << s.toStdString() << std::endl;
   //-----------^^^^^^^^^^^^^^^ crash
}

Change History (6)

comment:1 by gsherman, 18 years ago

Description: modified (diff)

comment:2 by g_j_m, 18 years ago

Owner: changed from gsherman to g_j_m
Status: newassigned

Resolved the given example in r5228. Haven't looked elsewhere for the same problem.

QString is implicitly shared, but I'm not sure if that resolves the problem for local variables returned via references (it does resolve the problem for QStrings returned via a copy).

Leaving this open as a reminder to look for and correct other instances of the same problem

comment:3 by g_j_m, 18 years ago

Resolution: fixed
Status: assignedclosed

The one in QgsFeature seems to be the only place where this is done, so I'm closing this ticket.

comment:4 by mloskot, 18 years ago

Yes, I dis some searching for places with similar problem and I've not found any. So, it seems to be the only such place.

comment:5 by anonymous, 17 years ago

Awaiting user input: unset
Must Fix for Release: No

comment:6 by (none), 15 years ago

Milestone: Version 0.8

Milestone Version 0.8 deleted

Note: See TracTickets for help on using tickets.