Is there a risk of crashing in the QImage::setColorSpace method?

[victorman]

When I was looking at the QT source code, I found that in the QImage::setColorSpace method, the validity of the d pointer is not determined after the execution of detach(). Is there a risk that this will cause a crash?

void QImage::setColorSpace(const QColorSpace &colorSpace)
{
    if (!d)
        return;
    if (d->colorSpace == colorSpace)
        return;
    if (!isDetached()) // Detach only if shared, not for read-only data.
        detach();
    d->colorSpace = colorSpace;
}

If this is a problem, are there any plans to fix it on QT5?

[Christian Ehrlicher]

Why do you think this can be a problem? Look into detach() and you will see that the d-pointer is always valid afterwards.

[victorman]

@Christian-Ehrlicher Thank you for your reply. Because in the QImage::setColor method there is a comment that says "In case detach() run out of memory".

void QImage::setColor(int i, QRgb c)
{
    if (!d)
        return;
    if (i < 0 || d->depth > 8 || i >= 1<<d->depth) {
        qWarning("QImage::setColor: Index out of bound %d", i);
        return;
    }
    detach();

    // In case detach() run out of memory
    if (!d)
        return;

    if (i >= d->colortable.size())
        setColorCount(i+1);
    d->colortable[i] = c;
    d->has_alpha_clut |= (qAlpha(c) != 255);
}

And I also looked at the code for detach(), where there's a line *this=copy(), if I run out of memory, does copy() return an empty image?

void QImage::detach()
{
    if (d) {
        if (d->is_cached && d->ref.loadRelaxed() == 1)
            QImagePixmapCleanupHooks::executeImageHooks(cacheKey());

        if (d->ref.loadRelaxed() != 1 || d->ro_data)
            *this = copy();

        if (d)
            ++d->detach_no;
    }
}

QImage QImage::copy(const QRect& r) const
{
    Q_TRACE_SCOPE(QImage_copy, r);
    if (!d)
        return QImage();

    if (r.isNull()) {
        QImage image(d->width, d->height, d->format);
        if (image.isNull())
            return image;

[Christian Ehrlicher]

So where should it get a nullptr? When there is no memory you will get a crash, Qt does not really care about oom.

[victorman]

@Christian-Ehrlicher But why doesn't QImage::setColorSpace check the validity of the d pointer after detach() like it does in QImage::setColor?

[Christian Ehrlicher]

The check in setColor() is not needed - if there is an oom the app will crash.