Trolltech Home | Qt-interest Home | Recent Threads | All Threads | Author | Date
All threads index page 3

Qt-interest Archive, December 2007
qPrintable() macro considered harmful


Message 1 in thread

This is a follow-up the Aug 07 thread "to how to print QString contents 
- QT4"

I've just spent days finding a really subtle problem with qPrintable(),
which is defined in include/QtCore/qglobal.h as:

#define qPrintable(string) (string).toLocal8Bit().constData()

This macro is dangerous. toLocal8Bit is defined as:

QByteArray QString::toLocal8Bit() const

NOTE that it's not returning a reference or pointer, so in the qPrintable
macro there is a temporary created.

The C++ language allows these temporaries to be destroyed as soon as 
they are
no longer needed. In this case, as soon as constData() returns. And in
it's destructor, QBtyeArray is deleting the data that constData() just
returned a pointer to!

In my nasty bug, that's exactly what was happening.

I should have been warned. On Fri, 31 Aug 2007 Konrad Rosenbaum wrote:

    What is the exact code you are using?

    This will work:
    QString mystring="hallo";
    qDebug("%s",mystring.toAscii().data());

    -> it creates a temporary byte array, extracts its data and destroys the
    temporary data after the command finishes

    This will work too:
    QString mystring="hallo";
    QByteArray myarray=mystring.toAscii();
    char *str=myarray.data();
    qDebug("%s",str);

    -> here the byte array is not temporary, so its data is guaranteed 
to exist
    for a while (until the byte array is destroyed or changed)

    This will NOT work:
    QString mystring="hallo";
    char *str=mystring.toAscii().data();
    qDebug("%s",str);

    -> it creates a temporary byte array, extracts its data, then 
destroys the
    temporary object, loses the data, and finally tries to display the lost
    data

I clearly didn't read this carefully enough. Konrad alludes to the problem.
However, I think there's even an error in the above. I don't think 
there's any
guarantee that this:

qDebug("%s",mystring.toAscii().data());

will work. The compiler is free to call the QBtyeArray destructor as soon
as data() returns.

The Trolltech documentation is of two minds on this. The "Porting to Qt 4"
document section on QString says:

    To obtain a const char * pointer to ASCII or Latin-1 data, use
    QString::toAscii() or QString::toLatin1() to obtain a QByteArray
    containing the data, then call QByteArray::constData() to access the
    character data directly. Note that the pointer returned by this
    function is only valid for the lifetime of the byte array; you should
    avoid taking a pointer to the data contained in temporary objects.

    QString greeting = "Hello";
    const char *badData = greeting.toAscii().constData(); // data is invalid

    QByteArray asciiData = greeting.toAscii();
    const char *goodData = asciiData.constData();

    In the above example, the goodData pointer is valid for the lifetime of
    the asciiData byte array. If you need to keep a copy of the data in a
    non-Qt data structure, use standard C memory allocation and string
    copying functions to do so before destroying the byte array.

Clearly stating the issue.

But the QString document says:

    You can pass a QString to a function that takes a const char * argument
    using the qPrintable() macro which returns the given QString as a const
    char *. This is equivalent to calling <QString>.toAscii().constData().

But clearly this isn't a good idea.

--
 [ signature omitted ] 

Message 2 in thread

On Friday 14 December 2007 19:59, Peter Hackett wrote:
> However, I think there's even an error in the above. I don't think
> there's any
> guarantee that this:
>
> qDebug("%s",mystring.toAscii().data());
>
> will work. The compiler is free to call the QBtyeArray destructor as soon
> as data() returns.

Actually, the C++ standard requires this to work. If the compiler destructs 
the object earlier than the end of the statement (ie. "before the ;") then it 
would be a compiler bug.


	Konrad

Attachment:

Attachment: pgpUdB8PomL8B.pgp
Description: PGP signature