user21025014
user21025014

Reputation: 11

Address boundary error - Why don't the nodes in my linked list get added properly?

I'm trying to implement a linked list in C, however I can't seem to figure out how to append elements to the list.

#include <malloc.h>
#include <stdio.h>
#include <stdlib.h>

typedef struct element {
    int value;
    struct element* next;
} element;

typedef struct list {
    element* head;
} list;

list* createList(int headValue) {
    list* L = malloc(sizeof(list));
    L->head = malloc(sizeof(element));
    L->head->value = headValue;
    L->head->next = NULL;
    return L;
}

// Returns the new length of the list
int appendToList(list* L, int value) {
    element* first = L->head;
    element* current = first;
    int length = 1;
    
    if (first == NULL) {
        printf("first is NULL\n");
        exit(2);
    }
    while (current != NULL) {
        current = current->next;
        length++;
    }
 
    current = malloc(sizeof(element));
    current->next = NULL;
    current->value = value;
    length++;

    return length;
}

// returns the value at index N (zero based)
//
// If the list has < N elements, returns -1 
int getAtIndexN(list* L, int N) {
    element* current = L->head;
    for (int i=0; i<N; i++) {
        if (current->next == NULL) {
            printf("unable to get to index %d\n", i+1);
            exit(1);
        }
        current = current->next;
    }
    return current->value;
}

int main(int args, char** argv) {
    int initvalues[] = {1,3,4,4};
    list* L = createList(5);
    for (int i=0; i<4; i++) {
        appendToList(L, initvalues[i]);
    }
    
    printf("element at index 1 has value %d\n", L->head->next->value);
    return 0;
}

I'm calling appendToList 4 times, in order to append the values from initvalues to the list, but only the head of the list seems to be added to the list correctly. the printf in main results in './a.out' terminated by signal SIGSEGV (Address boundary error). I'm not sure if the issue lies in appendToList or somewhere else.

Upvotes: 0

Views: 95

Answers (1)

user21025014
user21025014

Reputation: 11

In the loop while (current != NULL), current will eventually be null and you have lost the reference to the original list. Everything that follows, the allocation and assignments to current? happen in isolation and have no effect, other than leaking memory.

It seems that while (current != NULL) { should be while (current->next != NULL) { But there are more things to fix... Like current = malloc(sizeof(element)); should be current->next = malloc(sizeof(element)); and so on.

PS: IMO it's a bad design that createList adds a first node to the list. It should only create an empty list, i.e. L->head = NULL In other words, your code should IMO be able to handle an empty list. Your current code doesn't allow that.

Upvotes: 1

Related Questions