Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

Cannot work out why this segfaults



  • Hi

    I'm new to Qt and C++ but I have some knowledge in C.

    I keep getting an error 7 segfault whenever I try and call returnTestString(). Here is my code:

    TestClass.h

    @#include <QString>

    class TestClass
    {
    public:
    TestClass(QString testString);
    QString returnTestString() { return testString; }
    private:
    QString testString;
    };
    @

    Now, in my mainwindow.cpp, I have a slot that gets called and does this:

    @void MainWindow::setString(TestClass *tc)
    {
    this->test = tc;
    }@

    in mainwindow.h, 'test' is:

    private:
    TestClass *test;

    This works fine. But on a second slot / callback:

    @qDebug() << "The value of the test string is " + this->test->returnTestString(); @

    causes a segmentation fault. I'm not sure why.

    Thanks in advance!



  • Hi, and welcome.

    The forum will do a better job of formatting code if it is enclosed in @@. There's a button at the top left of the text input field that looks like a page with <> on it that spits them out.

    The code posted looks like it has suffered from a copy and paste error. There's a #endif, but no accompanying #ifdef

    In general, memory use violations aren't necessarily caused by the code running at the time that they're caught. Can you post a brief but complete example?



  • Hi Jeremy,

    I have updated my post so hopefully everything looks better now.

    It is actually this line: @ qDebug() << "The value of the test string is " + this->test->returnTestString();@ that is causing the crash. It is when I call the method I get a crash.

    I have check two things:

    • this->test is not NULL. I can get an address in memory with &this->test.
    • It is not the concatenation of strings that is causing this error. Just calling this->test->returnTestString(); on a separate line causes a crash.

    I have sent you a message.

    Thanks


  • Moderators

    Hi, and welcome to the Qt Dev Net!

    [quote]this->test is not NULL. I can get an address in memory with &this->test.[/quote]This alone isn't enough to prove that your pointer is OK. Uninitialized or dangling pointers can be non-NULL too.

    (If you haven't assigned anything to the pointer, it's uninitialized. If you assign an object to a pointer but then destroy the object, the pointer is dangling. These concepts apply to C pointers too.)

    How did you create your TestClass object?



  • I am worried the garbage collector is destroying the object or something.

    TestClass is created in a QList.

    @function QList<TestClass> *buildTests()
    {
    QList<TestClass> *tests = new QList<TestClass>();
    TestClass test = new TestClass("Hello World");
    tests->append(*test);
    return tests;
    }@

    I have a class later that takes this QList, iterates in a drop down menu. Later when the user activates the button clicked slot:

    @
    TestClass *testreturn;
    for (int i = 0; i < testList->count(); i++)
    {
    TestClass test= devicesList->at(i);
    if (test.returnTestString() == ui->testSelectionBox->currentText())
    {
    testreturn = &test;
    }
    // Later on:
    emit(test);
    @



  • [quote author="sn1994" date="1409651189"]I am worried the garbage collector is destroying the object or something.
    [/quote]

    Stop worrying! There is no garbage collector, unless you've added one or are using another framework that comes with one.

    I see that the QList contains TestClass objects, rather than pointers to the objects. Does TestClass implement a copy constructor and assignment operator? If not, I wonder if the default implementation is the problem.

    http://www.cplusplus.com/articles/y8hv0pDG/ describes what these are and why you might need to define customized versions.

    Alternatively, change the QList<TestClass> to QList<TestClass *>


  • Moderators

    [quote author="sn1994" date="1409651189"]
    @
    TestClass test= devicesList->at(i);
    //...
    testreturn = &test;
    @
    [/quote]
    test is a local variable. testreturn is a pointer that points to that local variable.

    test will be destroyed when it goes out of scope.



  • [quote author="JKSH" date="1409655317"][quote author="sn1994" date="1409651189"]
    @
    TestClass test= devicesList->at(i);
    //...
    testreturn = &test;
    @
    [/quote]
    test is a local variable. testreturn is a pointer that points to that local variable.

    test will be destroyed when it goes out of scope.[/quote]

    Good point. There's also the emit(test) which may not be what is intended.



  • Sorry, I am doing emit (testreturn). I had to make my code clean to post here, so that is just a mistake in posting.

    Should I make testreturn a public: variable and use memcpy to copy it? Oddly the first slot is OK with receiving the TestClass*, it is only later when I try and reference the global version I assign with this.test it crashes



  • p.s. I see your suggestion JKSH

    At the moment I have QList<TestClass> testlist*, do you mean change it to

    QList<TestClass*> testlist*

    or QList<TestClass*> testlist

    Thanks!


  • Moderators

    [quote author="sn1994" date="1409657238"]Sorry, I am doing emit (testreturn).[/quote]That doesn't work either: You need to emit a signal. What is the name of your signal?

    [quote]Should I make testreturn a public: variable and use memcpy to copy it?[/quote]No, avoid using public variables. Also avoid using low-level functions like memcpy and malloc -- use the C++ equivalents instead (e.g. std::copy, new)

    [quote author="sn1994" date="1409657238"]Oddly the first slot is OK with receiving the TestClass*, it is only later when I try and reference the global version I assign with this.test it crashes[/quote]It's still unclear to me what exactly your code is doing. Can you please post a minimal but complete example? Strip out all the parts that don't contribute to the segfault, and then post the full code for main(), MainWindow, and TestClass.



  • I can show you the proper code, I notice you are on Freenode. I tried to whois you but apparently you are not online.


  • Moderators

    You can post it here :) I haven't been on Freenode for a long time; I should probably update my profile.

    [quote author="sn1994" date="1409657846"]p.s. I see your suggestion JKSH

    At the moment I have QList<TestClass> testlist*, do you mean change it to

    QList<TestClass*> testlist*

    or QList<TestClass*> testlist

    Thanks![/quote]The suggestion actually came from jeremy_k. I believe he meant QList<TestClass*> testlist

    You almost never need to allocate a QList itself using new. It is an implicitly shared class, so we just copy it around or pass it by const-reference. See http://qt-project.org/doc/qt-5/implicit-sharing.html for details.



  • Yes, please. Post here. This allows future readers to benefit from the conversation. It also puts a little pressure on posters to write brief, on point samples.

    Confirmation on QList<TestClass *> testList.

    Having a list of pointers makes it clear that fetching an item, manipulating it, and then going out of scope:

    • will not create a new object,
    • will modify the original in the list
    • will not cause the object (original or copy of) to be destroyed

Log in to reply