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
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.