CODEWITHSUNDEEP
c++qt
Corvus
Corvus

Reputation: 63

Why does std::sort crash when working with a QList?

I get a couple different crashes running the below code, sometimes a segmentation fault and sometimes ASSERT: "&other != this" in file C:/Qt/5.12.3/mingw73_64/include/QtCore/qstring.h, line 958.

The code to reproduce this is below.

#include <QList>
#include <QDebug>

class Tag
{
public:
    Tag(const QString& name)
    {
        this->name = new QString(name);
    }

    Tag(const Tag& tag)
    {
        this->name = new QString(tag.getName());
    }

    ~Tag()
    {
        if (name != nullptr) delete name;
    }

    QString getName() const
    {
        return *name;
    }

    static bool sortByName(const Tag& t1, const Tag& t2)
    {
        return t1.getName() < t2.getName();
    }

private:
    QString* name;
};

int main(int argc, char **argv)
{
    QList<Tag> tags;

    Tag a("a");
    Tag b("b");
    Tag c("c");
    Tag d("d");
    Tag e("e");

    tags << e << c << a << b << d;

    qDebug() << "before sorting";
    for (Tag tag : tags) {
        qDebug() << tag.getName();
    }

    std::sort(tags.begin(), tags.end(), Tag::sortByName);

    qDebug() << "after sorting";
    for (Tag tag : tags) {
        qDebug() << tag.getName();
    }

    return 0;
}

I have noticed that if the initial QList is filled with pointers to Tag objects and sortByName() is changed to accept pointers instead, it does not cause issues. I'm at a loss as to why.

Upvotes: 1

Views: 595

Answers (1)

selbie
selbie

Reputation: 104539

The root of the problem is that you are using a raw pointer as a member variable (name) and trying to explicitly do a manual allocation/copy without the rule of 3 being satisfied. sort is invoking the copy assignment operator and getting the default implementation - which is to blindly copy the pointer value. Hence, double delete and crash...

The simplest fix is to just change name from a pointer to a concrete instance.

That is, this compiles and works fine with the main you already have provided.

class Tag
{
public:
    Tag(const QString& name) : _name(name)
    {
    }

    const QString& getName() const
    {
        return _name;
    }

    static bool sortByName(const Tag& t1, const Tag& t2)
    {
        return t1.getName() < t2.getName();
    }

private:
    QString _name;
};

If you actually need to keep name as a pointer, then add a copy assignment operator method to your Tag class:

class Tag
{
    …

    Tag& operator=(const Tag& t)
    {
        Qstring* oldName = this->name;
        this->name = new QString(t.getName());
        delete oldName;
        return *this;
    }
};

Upvotes: 4

Related Questions