CODEWITHSUNDEEP
clinked-listdoubly-linked-list
Jay Nguyen
Jay Nguyen

Reputation: 341

C - Read file to Doubly Linked List got segmentation fault

I am reading from txt file into a doubly linked list. The codes can do storing data into Nodes, but when I let it go through the linked list, it got a segmentation fault.

Could you guys please tell what has been wrong with the code, thank you!

This is the data structure:

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 to create linked list:

TelephoneBookList * createTelephoneBookList(char entry[]) {
    TelephoneBookList* aList = NULL;
    TelephoneBookNode* aNode = NULL;
    char *tokens;

    TelephoneBookNode *(*create)() = createTelephoneBookNode;
    aNode = (*create)();

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

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

    tokens = strtok(NULL, ", ");
    strcpy(aNode->telephone, tokens); //Fine until here

    //Do I need this line?
    //aList = (TelephoneBookList*) malloc(aList->size + 1) * sizeof aList);

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

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

TelephoneBookNode * createTelephoneBookNode() {
    TelephoneBookNode* aNode;

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

Upvotes: 0

Views: 43

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409166

//Do I need this line?
//aList = (TelephoneBookList*) malloc(aList->size + 1) * sizeof aList);

Yes. Yes you do need that line. Otherwise the next line

if (aList->head == NULL) {

will dereference a null pointer.

Though you already do that in the commented out malloc call, dereference a null pointer, with aList->size + 1.

The correct line should be

aList = malloc(sizeof *aList);

And since you create the list from scratch in the function, there is no need to check if it is empty or not, it will always be empty. More importantly, the malloc call will not initialize the memory it allocates, so using that memory (for example in an expression like aList->head == NULL) will lead to undefined behavior.

Allocate the list structure. And then initialize it as if it was empty. And don't forget to initialize the size member as well.

Upvotes: 2

Enno
Enno

Reputation: 1862

Your createTelephoneBookNode function does not initialize the node that it created. malloc() assigns it a memory block that's probably not been initialized with zeros, and as a result, the nextNode and previousNode pointers contain garbage. Either set them both to NULL, or allocate your memory with calloc().

Upvotes: 0

Related Questions