Reputation:
I am trying to read from a list that has 1 word per line and 20 lines, and then add each word to the end of a linked list. My problem is that when I print out the linked list at the end it is printing that last word in the file 20 times instead of each word once. I have been working on this for hours and can't figure out what i'm doing wrong.
In my main I have
while (!feof(input)) {
fscanf(input, "%s", currentName);
head = insert(head, currentName);
}
print(head);
delete(head);
Insert Function
node* insert(node* head, char* name) {
node *temp = NULL;
if (head == NULL) {
head = create_node(name);
}
else {
temp = head;
while(temp->next != NULL){
temp = temp->next;
}
temp->next = create_node(name);
}
return head;
}
Create Function
node* create_node(char* name) {
node *newNode;
newNode = malloc(sizeof(node));
if(newNode == NULL) {
printf("Failed to create node");
}
newNode->name = name;
newNode->next = NULL;
return newNode;
}
Print and Delete
void print(node* head){
while (head != NULL) {
printf("%s -> ", head->name);
head = head->next;
}
printf("NULL\n");
}
void delete(node* head) {
node *temp = NULL;
while(head != NULL) {
temp = head;
head = head->next;
free(temp);
}
}
Upvotes: 0
Views: 3862
Reputation: 66194
You're saving the address of the same buffer back in main()
for each and every insertion. Each node is simply holding the base address of currentName
, the content of which is changed with each input processed. Therefore you have a linked list of structures containing name
pointers, where each points to the same buffer (currentName
). Thus the last one will be the only one you see.
You need to dynamically allocate space for the name in create_node
. The following uses the POSIX function strdup()
to do this, though you're perfectly free to use a strlen/malloc
combination if you desire.
node* create_node(char* name)
{
node *newNode;
newNode = malloc(sizeof(node));
if(newNode == NULL)
printf("Failed to create node");
newNode->name = strdup(name);
newNode->next = NULL;
return newNode;
}
Don't forget when you're cleanup up your linked list to free()
each node name to avoid a memory leak.
void delete(node* head)
{
node *temp = NULL;
while(head != NULL)
{
temp = head;
head = head->next;
free(temp->name);
free(temp);
}
}
Unrelated: Your while-loop condition for loading your content is wrong. Read this answer to see why
Upvotes: 2