Creating List using pointers

Question

I want to create a linked list and keep two pointers. One for the head and one for the last element. I tried several approaches and just found a solution, but I don't know why my first solution don't work. The pointer in head->next always points on the last element, instead of the correct one.

int main(void){ // first solution

    struct pointList *lastElement;
    struct pointList *head;
    struct pointList headHelp = {.next = NULL, .p.x = 5, .p.y = 5};
    head = &headHelp;
    int i = 0;
    lastElement = &headHelp;
    for( i = 8; i < 10; i++){
        printf("LastElement: %d/%d\n", lastElement->p.x, lastElement->p.y);
        struct pointList *helpPoint;
        helpPoint = ((struct pointList*) malloc(sizeof(struct pointList)));
        struct pointList newElement = *helpPoint;
        newElement.next = NULL;
        newElement.p.x = i;
        newElement.p.y = i;

        lastElement->next = &newElement;
        lastElement = &newElement;
    }
    //printList(head);
    printf("LastElement: %d/%d\n", lastElement->p.x, lastElement->p.y);
    printf("head -> next: %d/%d\n", head->next->p.x, head->next->p.y);
    printf("finish\n");
    return 0;
}

output:

LastElement: 5/5
LastElement: 8/8
LastElement: 9/9
head -> next: 9/9
finish

but it should be : head -> next: 8/8 So the "next" pointer of head is changed in every loop execution

The solution which works looks like this:

int main(void){

    struct pointList *lastElement;
    struct pointList *head;
    struct pointList headHelp = {.next = NULL, .p.x = 5, .p.y = 5};
    head = &headHelp;
    int i = 0;
    lastElement = &headHelp;
    for( i = 8; i < 10; i++){
        printf("LastElement: %d/%d\n", lastElement->p.x, lastElement->p.y);
        lastElement->next = ((struct pointList*) malloc(sizeof(struct pointList)));

        lastElement->next->next = NULL;
        lastElement->next->p.x = i;
        lastElement->next->p.y = i;

        lastElement = lastElement->next;
    }
    //printList(head);
    printf("LastElement: %d/%d\n", lastElement->p.x, lastElement->p.y);
    printf("head -> next: %d/%d\n", head->next->p.x, head->next->p.y);
    printf("finish\n");
    return 0;
}

used struct:

struct pointList {
    struct point p;
    struct pointList *prev;
};

offtopic: Why are you mixing dynamic (malloc) memory with local memory in list nodes? — Mohit Jain, May 20 '15 at 08:34
This: `helpPoint = ((struct pointList*) malloc(sizeof(struct pointList)));` really should be `helpPoint = malloc(sizeof *helpPoint);`. Way shorter, easier to read, less risk of error, and just ... well, better. [Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). — unwind, May 20 '15 at 08:57
@Telijas Should there be struct pointList *next instead of struct pointList *prev; in the structure definition? — Vlad from Moscow, May 20 '15 at 09:05

score 1 · Answer 1 · answered May 20 '15 at 08:36

The problem with the first code snippet is that you take and use the pointer to a local variable. The variable newElement have its scope inside the loop only, so when the loop iterates the current newElement variable goes out of scope and you are left with a stray pointer. This leads to undefined behavior.

You also allocate memory for helpPoint but you don't actually use it (when you initialize newElement using helpPoint you initialize one uninitialized structure with another uninitialized structure), and you do not free it anywhere leading to memory leaks.

Creating List using pointers

1 Answers1