0

I am trying to create a linked list that will store words and how many times they occurred in the .txt file. Before reading I'm trying to create a linked list to see if it is okay. While testing, it is crashing.

#include <iostream>
#include <string>
struct n {

    std::string word;
    int occurance;
    n* next;
};

typedef n node;

int main() {
    node* root;
    root = (node*)malloc(sizeof(node*));
    
    root->word = "test";
    root->occurance = 5;

    std::cout << root->word
        << root->occurance << std::endl;
}

Error

Uğur Bilgin
  • 1
  • `typedef n node;` -- Totally unnecessary, and error-prone. Why did you need to do this? Second, what C++ book introduces using `malloc` to create a dynamically allocated object? That is wrong in this context – PaulMcKenzie Jan 15 '21 at 15:09
  • Why are you using `malloc` in C++ when you have `new`? – scohe001 Jan 15 '21 at 15:10
  • 1
    `root = (node*)malloc(sizeof(node*));` Your sizeof is a pointer. You want `sizeof(node)` – 001 Jan 15 '21 at 15:10
  • 1
    `root = (node*)malloc(sizeof(node*));` -- This does not construct `node` objects, but your code assumes it did. – PaulMcKenzie Jan 15 '21 at 15:13
  • @JohnnyMopp True, but even better to not bother with `malloc` at all, because it doesn't initialize anything. – Lukas-T Jan 15 '21 at 15:15
  • If you want a linked list in C++, use `std::list` . Rolling your own is literally pointless. At a bare minimum you should understand that all `malloc` does is allocate dynamic memory. That object class (`n`), is non-trivial. It has a member `word` that requires non-trivial construction, therefore so too goes `n`. Therefore, `node *root = new node;` would be appropriate, as `operator new` invokes construction, – WhozCraig Jan 15 '21 at 15:20

3 Answers3

1
root = (node*)malloc(sizeof(node*));

Is wrong in two ways. You should use

root = new node;

Firstly you code will allocate space for a node* (usually 4 or 8 bytes) not for a node.

Secondly malloc only allocates memory, but doesn't initialize it. See also In what cases do I use malloc and/or new?1 That means all members of the newly allocated node have indeterminate values and reading them will cause undefined behaviour. In your case this manifests as access vioaltion.

1 In modern C++ you should avoid both ;)

Lukas-T
  • 11,133
  • 3
  • 20
  • 30
0

malloc is a holdover from C, which is how C developed back in the previous century. It compiles, but that's mostly bad luck.

Just move to at least 1998, and use std::list, or perhaps std::map<std::string, int>. Explaining why obsolete technology fails isn't that useful.

MSalters
  • 173,980
  • 10
  • 155
  • 350
0

sizeof(node*) gets the size of a pointer to a node. Probably 4 or 8 bytes. malloc(sizeof(node*)) allocates that many bytes.

But a node is bigger than a pointer to a node. So then your code fills in all the data and goes past the end of the memory it allocated because it only allocated enough space for a pointer.

Solution 1: change malloc(sizeof(node*)) to malloc(sizeof(node))

Solution 2: change (node*)malloc(sizeof(node*)) to new node because it's C++ (not C) and you can do that.

Also, don't forget to free it (free(root); for malloc or delete node; for new). The OS automatically frees all your stuff when the program ends, so it's not important in this short program, but when you make a program that does a lot of stuff, you have to free the memory from one part so that the next part can reuse it.

user253751
  • 57,427
  • 7
  • 48
  • 90