CODEWITHSUNDEEP
clinked-list
yulai
yulai

Reputation: 761

C - Linked list - Insert element not updated - Only adding last input element

PREFACE: The goal is to prompt a user for input, adding each element (input line) into a linked list.

I have been playing around with some sample code from Learn-C.org, which shows a linked list example. I have modified the code so that it takes "strings" instead of integers.

My insert function is as follows: 

void push(node_t * head, char *data) {
    node_t * current = head;

    if(head == NULL) {
      printf("First element ever!\n");
    }
    else if(current->data == NULL) {
      current->data = data;
      current->next = NULL;

    }
    else {
      while (current->next != NULL) {
        current = current->next;
        }
      current->next = malloc(sizeof(node_t));
      current->next->data = data;
      current->next->next = NULL;
   }
}

Now, in MAIN, I initiate the list as follows: 

node_t * test_list = malloc(sizeof(node_t));

Adding elements is done with: 

  push(test_list, "FOO");
  push(test_list, "FEE");
  push(test_list, "FAA");

When printing the list, using print_list(test_list), I get the following output: 

FOO
FEE
FAA

PROBLEM

However, I have then included a while loop that prompts user for input and adds this to the linked list. 

char command[120];
int counter = 0;
while(counter < 3) {
    printf("Enter element: ");
    fgets((void *)command, sizeof(command), stdin);
    push(test_list, command);   //Insert
    counter++;
}

However, this does not add each element into the link list. Instead, it adds the LAST element into the list three times.

For instance, when providing: 

Enter element: Argentina
Enter element: Mexico
Enter element: Sweden

The list prints out as: 

FOO
FEE
FAA
Sweden
Sweden
Sweden

EDIT (added print function)

My print function is as follows: 

void print_list(node_t * head) {
    node_t * current = head;

    printf("**** Printing list ****\n");
    while (current != NULL) {
        printf("%s\n", current->data);
        current = current->next;
    }
}

What am I missing, and alternatively: How can I fix this? Any help is highly appreciated.

Upvotes: 1

Views: 172

Answers (3)

Nayan Khant
Nayan Khant

Reputation: 1

current->data = data; here you are copy only pointer address not data, on that address (address of 'command') last data ('Sweden') will be available. you should use strcpy(current->data,data) to copy data.

Upvotes: 0

rohit89
rohit89

Reputation: 5773

Use strdup to return a copy of the string allocated on the heap.

The strdup() function returns a pointer to a new string which is a duplicate of the string s. Memory for the new string is obtained with malloc(3), and can be freed with free(3).

node_t *test_list = malloc(sizeof(node_t));
test_list->next = NULL;
test_list->data = NULL;
while(counter < 3) {
  printf("Enter element: ");
  fgets((void *)command, sizeof(command), stdin);
  push(test_list, strdup(command));   //Insert
  counter++;
}

Upvotes: 2

Sami Kuhmonen
Sami Kuhmonen

Reputation: 31203

You have an array command with capacity of 120. You use this to read in values. This is ok.

Then you send a pointer to this array to be stored. It gets stored, everything is still fine.

The next time you read input, you read it to the same array, for which you gave a pointer to be stored. So you are changing the content of the memory where that pointer points to. This is not ok.

You need to allocate separate memory areas for each string and handle their deallocations. strdup is the easiest way to get a new memory block with the contents of the user input.

But do remember that you really have to deallocate the memory when you don't need it anymore. In this case you might never remove any strings, but when you do, you can't just remove the element, you have to also free the memory used by the string.

Upvotes: 0

Related Questions