About QString's empty string allocation inconsistency

dldd

Consider the following cases for Qt 5.13.1:

QString a;               // 1
QString b("");           // 2
QString::fromUtf8("");   // 3
QString::fromLatin1(""); // 4

It seems that QString does dynamic memory allocation in cases 2 and 3, but not in cases 1 and 4. This seems peculiar:

Why to allocate for empty string in the first place?
Why to allocate only for some kind of empty strings?

As for the rationale for this question, was going through some code which had something like this:
QString::fromUtf8(someFunctionReturningCharPtrToUtf8Data());
This could get called quite often and the function may frequently return empty string, so it would seem reasonable that QString always handled the empty string case without redundant allocations so that user wouldn't need to work around it manually.

References:
qstring.h (5.13.1)
qstring.cpp (5.13.1)
qutfcoded.cpp (5.13.1)
qarraydata.cpp (5.13.1)

VRonin

@dldd said in About QString's empty string allocation inconsistency:

does dynamic memory allocation in cases 2 and 3, but not in cases 1 and 4

Case 2 calls Case 3 internally so they are the same. But you are right. fromUtf8 calls QString::QString(int size, Qt::Initialization) that misses the check for lenght size that is done in other constructors. We can call this a bug. Feel free to submit a patch.

What it is vs what it should be:

QString::QString(int size, Qt::Initialization)
{
    if (size <= 0) {
        d = Data::allocate(0);
    } else {
        d = Data::allocate(size + 1);
        Q_CHECK_PTR(d);
        d->size = size;
        d->data()[size] = '\0';
    }
}

SGaist

Hi,

For that kind of low level details, you better ask on the interest mailing list. You'll find there Qt's developers/maintainers. This forum is more user oriented.

JKSH

I don't know the reasons for the internal designs, but note that Case #1 is a null string, while cases #2-#4 are empty strings. See QString::isNull() and QString::isEmpty()

VRonin

@dldd said in About QString's empty string allocation inconsistency:

does dynamic memory allocation in cases 2 and 3, but not in cases 1 and 4

Case 2 calls Case 3 internally so they are the same. But you are right. fromUtf8 calls QString::QString(int size, Qt::Initialization) that misses the check for lenght size that is done in other constructors. We can call this a bug. Feel free to submit a patch.

What it is vs what it should be:

QString::QString(int size, Qt::Initialization)
{
    if (size <= 0) {
        d = Data::allocate(0);
    } else {
        d = Data::allocate(size + 1);
        Q_CHECK_PTR(d);
        d->size = size;
        d->data()[size] = '\0';
    }
}

dldd

Thanks for the responses. While the adjustment itself is expected to be short, editing such a core part of Qt as QString's constructor is something I'll leave to real Qt developers. But I'll considered creating a ticket to Qt bug tracker.

SGaist

@dldd said in About QString's empty string allocation inconsistency:

Thanks for the responses. While the adjustment itself is expected to be short, editing such a core part of Qt as QString's constructor is something I'll leave to real Qt developers.

Why ? As you said, it's a small modification. This is a good way to get started contributing :-)

You'll be a "real Qt developers" as other contributors. The fact that this is a core class shouldn't make you afraid of fixing it. On the contrary, you'll have a nice impact for other Qt users.

dldd

@SGaist said in About QString's empty string allocation inconsistency:

Why ? As you said, it's a small modification. This is a good way to get started contributing :-)

You'll be a "real Qt developers" as other contributors. The fact that this is a core class shouldn't make you afraid of fixing it. On the contrary, you'll have a nice impact for other Qt users.

Because "real Qt developers" have better insight on matters :) As demonstrated in this case: the bug report QTBUG-80808 was closed as "Won't Do" and I don't think the resolution would have been different had there been a patch. While I don't quite understand why one would like to allocate for empty string when there already are mechanisms used to avoid it, I'm not in the position to start questioning it.

aha_1980

@dldd said in About QString's empty string allocation inconsistency:

While I don't quite understand why one would like to allocate for empty string when there already are mechanisms used to avoid it, I'm not in the position to start questioning it.

Most often the answer is: binary compatibility, which is guaranteed for all 5.x versions and can be broken by 6.0.

As Thiago said, there are plans to change QString within the next year, so stay tuned, as I do.

Regards

dldd

@aha_1980 said in About QString's empty string allocation inconsistency:

Most often the answer is: binary compatibility, which is guaranteed for all 5.x versions and can be broken by 6.0.

As far as I understand, the fix (e.g. the one demonstrated by VRonin) would not break binary compatibility