0

I'm wring my linked list implementation in C and I am facing the problem that linked list doest not add elements to itself. It only prints the first element.
Something is wrong here and I can't figure out what.
I managed to localize the problem that first element next field is always NULL no matter what I do. All the other things are legal. I add the results of the running at the end.

Here is linked list declaration:

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

Here is usage:

void checkLinkedList()
{
    list_element list = createNewLinkedList();
    list.value = 100;

    for (int i = 1; i < 10; i++) {            
        printf("added another\n");

        int value = rand();
        insertNewElementAtEndWithValue(list, value);
    }
}

void insertNewElementAtEndWithValue(list_element element, int value)
{       
    printf("Starting element address: %p\n", &element);

    list_element *myElement = &element;       
    int shouldContinue = 1;

    while (shouldContinue) {            
        if (myElement->next == NULL) {
            printf("next is null, value is %d \n", value);

            list_element *new = (list_element *)malloc(sizeof(list_element));
            printf("new address %p\n", new);
            new->value = value;
            new->next = NULL;
            myElement->next = new;
            printf("New is: %p, myElement->next is: %p\n", new, myElement->next);
            shouldContinue = 0;
        } else {
            printf("Proceeding to next");
            myElement = myElement->next;
        }
    }
}

Results:

./main

added another
Starting element address: 0x7fff5c10aac0
next is null, value is 16807 
new address 0x7fa3d3504b60
New is: 0x7fa3d3504b60, myElement->next is: 0x7fa3d3504b60
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 282475249 
new address 0x7fa3d3504b70
New is: 0x7fa3d3504b70, myElement->next is: 0x7fa3d3504b70
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 1622650073 
new address 0x7fa3d3504b80
New is: 0x7fa3d3504b80, myElement->next is: 0x7fa3d3504b80
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 984943658 
new address 0x7fa3d3504b90
New is: 0x7fa3d3504b90, myElement->next is: 0x7fa3d3504b90
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 1144108930 
new address 0x7fa3d3504ba0
New is: 0x7fa3d3504ba0, myElement->next is: 0x7fa3d3504ba0
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 470211272 
new address 0x7fa3d3504bb0
New is: 0x7fa3d3504bb0, myElement->next is: 0x7fa3d3504bb0
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 101027544 
new address 0x7fa3d3504bc0
New is: 0x7fa3d3504bc0, myElement->next is: 0x7fa3d3504bc0
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 1457850878 
new address 0x7fa3d3504bd0
New is: 0x7fa3d3504bd0, myElement->next is: 0x7fa3d3504bd0
added another
Starting element address: 0x7fff5c10aac0
next is null, value is 1458777923 
new address 0x7fa3d3504be0
New is: 0x7fa3d3504be0, myElement->next is: 0x7fa3d3504be0
Number of elements in list: 1
 100,
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
Dvole
  • 5,725
  • 10
  • 54
  • 87
  • 3
    Hint: C has "pass by value" semantics. – juanchopanza Feb 15 '15 at 23:25
  • @juanchopanza thanks, changed argument to pointer and it worked. – Dvole Feb 15 '15 at 23:29
  • @Dvole also, [do not cast `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) and always check the return value of `malloc()`. – Iharob Al Asimi Feb 15 '15 at 23:49
  • I expanded my comment into an answer for completeness. – juanchopanza Feb 16 '15 at 08:20
  • 1
    I would suggest using 2 more pointers in the list data structure: 1. head element, always points at the first element; 2. tail element, which always points at the last element; it makes the time complexity O(1) in every case except when deleting the last but one element (you need to fix the tail pointer, find it iteratively/recursively - O(n)). In the case of insertion, you simply add at the end and, again, fix the tail pointer – Mark Feb 16 '15 at 08:40

1 Answers1

1

The issue is that C has pass by value semantics, so in the body of this function

void insertNewElementAtEndWithValue(list_element element, int value) {...}

both element and value are local copies of the arguments.

In your case, you intend to modify the first argument, for which you need referential semantics. This can be achieved by using a pointer instead:

void insertNewElementAtEndWithValue(list_element* element, int value)
{
  /* as before except element is a pointer */
}
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • Just for the sake of information, I used to do this by returning the new list head and assigning that in the caller. Different approach, same result. – SukkoPera Feb 16 '15 at 08:22
  • @SukkoPera That's a very good point. It makes the semantics of the function clearer and in fact my preferred way of doing things. – juanchopanza Feb 16 '15 at 08:24