-1

I'm trying to replicate a Binary Tree where the Nodes have a struct in its Node class. I can't seem to figure why it crashes when setting the bookName string. It keeps on crashing. Going to debug mode, it says it can't see the reference number or can't read the string.

#include <iostream>
#include <string>
using namespace std;

struct Book {
    string name;
};
struct TreeNode {
    float key;
    Book book;
    TreeNode* leftPtr;
    TreeNode* rightPtr;
};
class MyTree {
    TreeNode* rootPtr;
    // this function crashes!
    TreeNode* makeNode(float key,string bookName) {
        TreeNode* newNode = (TreeNode*)malloc(sizeof(TreeNode));
        if (newNode == NULL) {
            cout << "Error: No more space";
        }
        else {
            newNode->key = key;
            cout << newNode->book.name << endl;
            newNode->book.name = bookName;// CRASHES HERE
            cout << "Name: " << newNode->book.name;
            newNode->leftPtr = NULL;
            newNode->rightPtr = NULL;
        }
        return newNode;
    }
    TreeNode* insert(TreeNode* nodePtr, float key, string bookName) {
        if (nodePtr == NULL) {
            nodePtr = makeNode(key, bookName);
        }
        else if (key < nodePtr->key) {
            nodePtr->leftPtr = insert(nodePtr->leftPtr, key, bookName);
        }
        else if (key > nodePtr->key) {
            nodePtr->rightPtr = insert(nodePtr->rightPtr, key, bookName);
        }
        else { cout << "Error: Key already exist. Cannot add node."; }
        return nodePtr;
    }
public:
    MyTree() { rootPtr = NULL; }
    void addNode() {
        // ask for input
        float key = 0; string bookName = "default";
        cout << "Enter a value for the key: ";
        cin >> key;
        cout << "Enter the name for the book: ";
        cin >> bookName;

        // insert
        rootPtr = insert(rootPtr, key, bookName);
    }
};
int main() {
    MyTree a;
    a.addNode();
}

The debug error message is this:

Exception thrown at 0x0121DA2E in Assessment.exe: 0xC0000005: Access violation reading location 0xCDCDCDCD.

Please help


Edit 1: Replaced the malloc as suggested but it crashes when setting the key variable.

TreeNode* makeNode(float key,string bookName) {
        cout << "Here";
        TreeNode* newNode = new TreeNode;// CRASHES HERE
        cout << "Here";
        if (newNode == NULL) {
            cout << "Error: No more space";
        }
        else {
            newNode->key = key;
            newNode->book.name = bookName;
            cout << "Name: " << newNode->book.name;
            newNode->leftPtr = NULL;
            newNode->rightPtr = NULL;
        }
        return newNode;
    }
CraftedGaming
  • 499
  • 7
  • 21
  • The exception means that you read through an uninitialized pointer. – πάντα ῥεῖ Jun 05 '19 at 06:18
  • 1
    `new` does not return a null pointer, so no need to check. (And it's spelled `nullptr` for quite some time now). – n. m. could be an AI Jun 05 '19 at 06:33
  • 1
    Remember that output to `std::cout` is *buffered*, so unless you flush the output (with e.g. `std::endl`) you can't reliably use it for locating crashes. Use an actual debugger to run the program and catch the crash "in action". – Some programmer dude Jun 05 '19 at 06:34
  • 1
    [Cannot reproduce the crash](https://ideone.com/TafL8T). Please make sure you are running the program you think you are running. Also, when you post code, post all of it, do not omit `#include` directives and such. – n. m. could be an AI Jun 05 '19 at 06:39

2 Answers2

3

If you search a little for the pattern 0xCDCDCDCD you will quickly find out that it's what the debug-build of malloc uses to fill the memory it allocates. That in turn means you use uninitialized memory in your program. You allocate memory with malloc but you don't initialize the memory.

And that allocation with malloc is the root cause of your problem. The malloc function allocates memory, but it doesn't call object constructors. That in turn means that the std::string object of the Book structure is not constructed, and using it in any way leads to undefined behavior.

The solution, beyond reading good C++ books and learn C++ properly, is to use the new operator to allocate your object:

TreeNode* newNode = new TreeNode;
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1
TreeNode* newNode = (TreeNode*)malloc(sizeof(TreeNode));

Don't do this. Your TreeNode contains an std::string, which needs to be properly initialized. With malloc you don't initialize anything, you just allocate memory. So when you try to use the string, it's most likely in some invalid state and will cause nothing but trouble. There aren't many reasons to use malloc in C++, better just do this instead:

TreeNode* newNode = new TreeNode;

And when it's not needed anymore, do delete newNode;.

Blaze
  • 16,736
  • 2
  • 25
  • 44