C - error when allocating dynamic memory for linked list node

Question

I am trying to write a code that reads a text file to a linked list and stores in memory. I don't know why there are errors on malloc functions I use it in my code.

This is the pre-given header file that need to keep as is:

#ifndef ADDRESSBOOK_LIST_H
#define ADDRESSBOOK_LIST_H

#define NULL_SPACE 1

#define NAME_LENGTH (20 + NULL_SPACE)
#define TELEPHONE_LENGTH (10 + NULL_SPACE)

typedef struct telephoneBookNode
{
    int id;
    char name[NAME_LENGTH];
    char telephone[TELEPHONE_LENGTH];
    struct telephoneBookNode * previousNode;
    struct telephoneBookNode * nextNode;
} TelephoneBookNode;

typedef struct telephoneBookList
{
    TelephoneBookNode * head;
    TelephoneBookNode * tail;
    TelephoneBookNode * current;
    unsigned size;
} TelephoneBookList;

This is the code I write to load txt file into memory:

The entry has a format ID, Name, Number like 123, Alice, 0123456789

#include "addressbook_list.h"
TelephoneBookList * createTelephoneBookList(char entry[])
{
    TelephoneBookList* aList = NULL;
    TelephoneBookNode* aNode = createTelephoneBookNode();
    char *tokens;

    tokens = strtok(entry, ", ");
    aNode->id = tokens;

    tokens = strtok(NULL, ", ");
    aNode->name = tokens; //Error: array type char[21] is not assignable

    tokens = strtok(NULL, ", ");
    aNode->telephone = tokens; //Error; array type char[11] is not assignable

    aNode->nextNode = aList->head;
    aList->head = aNode;

    if (aList == NULL)
    {
        aNode->nextNode = NULL;
        aNode->previousNode = NULL;

        aList->current = aNode;
        aList->head = aNode;
        aList->tail = aNode;
    }
    else
    {
        aList->tail->nextNode = aNode;
        aNode->nextNode = NULL;

        aList->tail = aList->tail->nextNode;
    }

    return aList;
}

This is the function to create nodes, I got error:

incompatible pointer to integer conversion assigning to 'char' from 'char*', dereferenced with *

TelephoneBookNode * createTelephoneBookNode()
{
    TelephoneBookNode* aNode;

    aNode = (TelephoneBookNode*) malloc(sizeof aNode);

    aNode->id = (int) malloc(sizeof aNode->id);
    aNode->name = (char*) malloc(sizeof aNode->name);
    aNode->telephone = (char*) malloc(sizeof aNode->telephone);

    return aNode;
}

Please someone can explain me a bit on the error. Thank you very much!

Note: They say [you shouldn't cast the result of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — MikeCAT, May 05 '16 at 04:24
Isn't aNode->id an integer? How can one assign heap memory to an integer? — Adrian M., May 05 '16 at 04:28
``aNode->id = (int) malloc(sizeof aNode->id);`` youhad a java class before, did you? You do not need to malloc integral values in the struct. An int is an integral value. — BitTickler, May 05 '16 at 04:30
@AdrianM. By covering up the (not so pointless after all) warnings about pointer to integral conversion with that hideous `(int)` cast. — WhozCraig, May 05 '16 at 04:30
``aNode = (TelephoneBookNode*) malloc(sizeof aNode);`` aNode is a pointer and its size is certainly not the same as that of a ``TelephoneBookNode`` — BitTickler, May 05 '16 at 04:32
@AdrianM. which brings me to a word of advice for the OP. Casting in C is rarely needed, and even then only in the hands of someone that *knows* the language, *seriously* understands what they're doing, and humble enough to admit fallibility. . For someone new to the language, it should be avoided like a plague of death. The requirements and language resources needed for dare-I-say-all learning-level projects shouldn't need it. If you find yourself writing code that doesn't compile without it, take it as a strong hint you're probably doing something very, *very* wrong. — WhozCraig, May 05 '16 at 04:38
There is also a bug in ``createTelephoneBookList`` regarding variable ``aList``. — BitTickler, May 05 '16 at 04:39
Small quibble "An int is an *integer* value". An *integral* value would be "essential to completeness" or "very important and necessary" — David C. Rankin, May 05 '16 at 04:58
@WhozCraig, something tells me you are not entirely a programmer by trade or education -- far too much witty linguistic flourish in your comment... — David C. Rankin, May 05 '16 at 05:01
You're attempting to allocate memory for `aNode->name` and `aNode->telephone`, where you've already predefined their size in the struct (i.e. no allocation is necessary). Also, `malloc(sizeof aNode)` will allocate 4 bytes on 32 bit systems, 8 on 64 bit; this is the size of a pointer. What you want is `malloc(sizeof TelephoneBookNode)` or `malloc(sizeof *aNode)` <-- I've seen this notation, but I don't use it, I always use the struct name, even if it may be less safe. — yapdog, May 05 '16 at 07:16

score 4 · Accepted Answer · edited May 23 '17 at 10:33

They say you shouldn't cast the result of malloc() in C.
You cannot assign to arrays, which will be converted to non-lvalue pointer when used as operand of = operator, in C.
TelephoneBookNode has two pointers and some other members, but you allocated space for only one pointer. This will cause luck of space and out-of-range access in typical environment.
Allocating a memory, converting it to some integer in implemention-defined manner and using it to initializing a menber looks weird.

Your createTelephoneBookNode() function should be like this:

TelephoneBookNode * createTelephoneBookNode()
{
    TelephoneBookNode* aNode;

    /* add dereference operator to get the size of what will be pointed by aNode */
    aNode = malloc(sizeof *aNode);

    /* do initialization of member if it is required */

    return aNode;
}

strcpy() function is available in string.h to copy strings, and atoi() function is available in stdlib.h to convert strings into integers. Using these, the part that is assigning data to members should be like this:

tokens = strtok(entry, ", ");
aNode->id = atoi(tokens);

tokens = strtok(NULL, ", ");
strcpy(aNode->name, tokens);

tokens = strtok(NULL, ", ");
strcpy(aNode->telephone, tokens);

Note that error checking is omitted here. Add them to make the program safer.

score 0 · Answer 2 · edited May 05 '16 at 04:58

You do not need to malloc for following:

aNode->id = (int) malloc(sizeof aNode->id);
aNode->name = (char*) malloc(sizeof aNode->name);
aNode->telephone = (char*) malloc(sizeof aNode->telephone);

As memory is already allocated when you declared them . You just need to allocate memory for link list node ,like:

TelephoneBookNode * createTelephoneBookNode()
{
    Telephonenter code here`eBookNode* aNode;

    aNode = (TelephoneBookNode*) malloc(sizeof aNode);

    return aNode;
}

score 0 · Answer 3 · answered May 06 '16 at 06:09

0

int id is an integer and not a pointer, so no need to malloc.
Both are char arrays of fixed size, No need to malloc for it.
Only for a node you can do a malloc and thats sufficient.

answered May 06 '16 at 06:09

techEmbedded

2
1
3

C - error when allocating dynamic memory for linked list node

3 Answers3