1

below is the code for load file .On button click event the load function is called which return XMLElement pointer variable.(a valid address is returned) but unable to access the members of XMlElement using ->operator because segmentation fault occurs.

XMLElement *XMLUtilities::load(string filepath)
{
    XMLDocument doc;
    char *cstr = new char[filepath.length() + 1];
    strcpy(cstr, filepath.c_str());
    XMLError err=doc.LoadFile(cstr);
    XMLElement *root=nullptr;
    if(err==XML_ERROR_FILE_NOT_FOUND)
    {
        return nullptr;
    }
    else
    {
         root=doc.FirstChildElement();
         cout<<root->Name();
        return root;
    }

}
Below is the code for button click..

`void MainWindow::on_pushButton_clicked()
{
   XMLUtilities util;
   QString filepath=QFileDialog::getOpenFileName(this,"open A file","C://");
   string str=filepath.toStdString();
   XMLElement *doc=util.load(str);
   cout<<&doc;   **/prints a address location **
   cout<<doc->Name();  **/segmentation fault occurs**
   if(doc)

   {

       QMessageBox::information(this,"success",filepath);
      // util.traverse(root);
   }
   else
       QMessageBox::information(this,"fail",filepath);


}
  • 2
    `doc` is a local object on stack. When the method finishes it will be destroyed. Most likely all objects inside are also destroyed and you return one. It’s not valid outside. – Sami Kuhmonen Sep 02 '18 at 11:03
  • Try `cout<<(nullptr == doc);` please, I don't very understand the purpose of `&doc`. – JustWe Sep 03 '18 at 06:39
  • In add. to @Jiu: `XMLElement *doc=util.load(str);` stores the return of `util.load(str);` into a pointer variable (a variable with type `XMLElement*`). To access, the pointer you have to use `doc`. (To access the _contents of_ pointer, you have to use `*doc`.) `&doc` provides the address of the pointer variable (which is local i.e. allocated on stack). It has type `XMLElement**` (pointer to pointer to `XMLElement`). As allocation on stack can hardly fail (except you have _stack overflow_), this addr. will be always valid in the block scope but it's surely worthless for what you want to check. – Scheff's Cat Sep 03 '18 at 07:12

1 Answers1

1

AS @Sami Kuhmonen pointed out in a comment, the problem is that when the method MainWindow.on_pushButton_clicked() is finished, all local variables are destroyed, including doc. This destroys all the nodes, elements... and so on inside the document, including of course the root node.

The simplest solution would be to return the document instead of just the root element.

XMLDocument XMLUtilities::load(string filepath)
{
    XMLDocument doc;
    // ...
    return doc;
}

Unfortunately for this example, this is not possible, since the authors of tinyxml2 considered that it would be inefficient to allow copying a whole document in memory (which is a good thing).

The only possibility I can think of is to actually read the XML in XMLUtilities.load(), and return a pointer to the root object of your own classes, not XMLNode or XMLElement.

For example, if you were reading information about cars, as in:

<cars>
    <car plate="000000">
        <owner ...
    </car>
    ...
</cars>

You would return a pointer to the class CarsList, which would represent the root element cars. Following your code, this pointer would be nullptr in case the file was not found or the data could not be retrieved.

Hope this helps.

Baltasarq
  • 12,014
  • 3
  • 38
  • 57