scanf() doesn't work in malloc struct members

Question

I define a struct data type:

typedef struct LinkNode LinkNode;
struct LinkNode {
    char *name;
    LinkNode *next;
};

And call it in main(): Example1:

int main()
{
    LinkNode *pnode = (LinkNode *) malloc(sizeof(LinkNode));

    scanf("%s", pnode->name);
    ...
    free(pnode);
    return 0;
}

It doesn't work, unless add one line: Example 2

int main()
{
    LinkNode *pnode = (LinkNode *) malloc(sizeof(LinkNode));
    pnode->name = (char *) malloc(sizeof(char));

    scanf("%s", pnode->name);
    ...
    free(pnode);
    return 0;
}

But the following code works: Example 3

int main()
{
    LinkNode *pnode = (LinkNode *) malloc(sizeof(LinkNode));

    pnode->name = "Jim";
    ...
    free(pnode);
    return 0;
}

So what's the problem? I have been assigned memory to pnode, why scanf() doesn't work? I'm using VS2010. Thanks!

Plus as you've found name is a pointer so you'll need to allocate memory seperately as you've done (with fix as highlighted by billz). The original code is only allocating room for the pointer itself, not any data. — PeterJ, Dec 17 '12 at 02:54
@fishiwhj Are you allowed to use C++ libraries, for example std::string? — billz, Dec 17 '12 at 03:04
@billz Of course, I hope you could show more details here :-) — fishiwhj, Dec 17 '12 at 06:07

imreal · Accepted Answer · 2012-12-17T03:08:30.093

When you use scanf, you need an allocated block of memory to write to. When you allocate memory for name with malloc it works, even if you are just allocating one byte (like @billz pointed out), because you have a valid memory address now. You have to allocate enough memory though, or you will overwrite something and cause trouble sooner or later.

pnode->name = (char *) malloc(sizeof(char) * <max size of input>);

When you assign a string literal to the pointer name (pnode->name = "Jim";), you are merely setting it to the address of a hardcoded string in memory. If you try to modify it bad things will happen, so never do this unless you are using a const pointer.

Bottom line, name is just a pointer, to do anything with it, you need it to point to something valid.

score 2 · Answer 2 · answered Dec 17 '12 at 03:07

When you allocate memory for the structure, it only creates memory for the entire node but not the pointer (name) that is used within. So you have to allocate memory for it explicitly. 1. you did a malloc for pnode->name, allocating memory and then try writing to it 2. pnode->name = "some string", by doing this you are allocating memory and storing the value in it. Hence these snippets work.

score 1 · Answer 3 · answered Dec 17 '12 at 03:08

scanf() has been around for decades. What do you think is the problem? Your code or scanf? :).

In the first example, LinkNode *pnode = (LinkNode *) malloc(sizeof(LinkNode)); allocates a LinkNode object, but does not initialize the char *name. Hence since its a memory location, it contains a junk address until it has been assigned.

There are three correct ways to assign a `char*`:
1. Create dynamic memory space and then assign it to the `char *`
2. Assign it to a constant string such as "Jim" in your example
3. Assign the pointer to another pointer that has been created by either 1 or 2

And this is exactly what you do. In your 2nd Example, you create space for your string data, and then assign it to pnode->name.

In your 3rd example, you assign pnode->name to a constant string.

This is how C pointers work.

Now coming to scanf(): it will read input until a whitespace is encountered, and assign it, formatted, to a character array buffer. Hence you will need a stack-initialized character array, or dynamically created character array(using malloc)

score 1 · Answer 4 · answered Dec 17 '12 at 06:11

1

As you can use C++, I'd like to suggest C++ way. See my sample code and comment

#include <string>
struct LinkNode {
    std::string name;    // let string manage dynamic memory instead of raw pointer
    LinkNode *next;
};     

int main()
{
  LinkNode *pNode = new LinkNode;  // use new instead of malloc
  std::cin >> pNode->name;         // use std::cin instead of scanf    
  delete pNode;
  return 0;
}

answered Dec 17 '12 at 06:11

billz

44,644
9
83
100

WOW, I see. C++ way is cleaner, I will try to learn it, thanks! – fishiwhj Dec 17 '12 at 06:35

score 0 · Answer 5 · answered Dec 17 '12 at 03:09

LinkNode consists of two pointers. In a 32-bit architecture, sizeof(LinkNode) equals 8 bytes (4 bytes for each pointer).

When you get memory for your struct, you will get enough space for two pointers, but any data it points to will have to have its separate memory allocated. Therefore the correct way to do what you want is to do two mallocs. One for the struct, and one for the internal string buffer.

You can also have a static buffer by doing

typedef struct LinkNode LinkNode;
struct LinkNode {
    char name[256];
    LinkNode *next;
};

In which case, allocating memory for LinkNode, will get enough memory for the internal buffer as well.

scanf() doesn't work in malloc struct members

5 Answers5