Can not read the string variable in linked list

Question

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

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
– PaulMcKenzie, Commented Jan 15, 2021 at 15:09
root = (node*)malloc(sizeof(node*)); Your sizeof is a pointer. You want sizeof(node) — 001
– 001, Commented Jan 15, 2021 at 15:10
root = (node*)malloc(sizeof(node*)); -- This does not construct node objects, but your code assumes it did. — PaulMcKenzie
– PaulMcKenzie, Commented Jan 15, 2021 at 15:13
@JohnnyMopp True, but even better to not bother with malloc at all, because it doesn't initialize anything. — Lukas-T
– Lukas-T, Commented Jan 15, 2021 at 15:15

Lukas-T · Accepted Answer · 2021-01-15 15:19:54Z

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?¹ 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.

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

answered Jan 15, 2021 at 15:19

Lukas-T

11.4k4 gold badges24 silver badges32 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

MSalters · Accepted Answer · 2021-01-15 15:11:41Z

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.

answered Jan 15, 2021 at 15:11

MSalters

182k11 gold badges171 silver badges376 bronze badges

Comments

Stack Exchange Broke The Law · Accepted Answer · 2021-01-15 15:17:06Z

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.

answered Jan 15, 2021 at 15:17

Stack Exchange Broke The Law

59.2k7 gold badges57 silver badges103 bronze badges

1 Comment

PaulMcKenzie Over a year ago

The other issue is that node is not a trivial type, thus malloc is a non-starter.

Collectives™ on Stack Overflow

Can not read the string variable in linked list

3 Answers 3

Comments

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related