Why Does Every Node in My Linked List Have the Same Data from a Text File in C?

  • Thread starter Thread starter Hallingrad
  • Start date Start date
  • Tags Tags
    List
AI Thread Summary
The discussion revolves around troubleshooting a linked list implementation in C, where the last line read from a text file is being incorrectly assigned to every node. Key issues identified include the incorrect handling of memory allocation for new nodes, specifically the zeroing out of the pointer immediately after allocation, which discards the allocated memory. Suggestions emphasize the importance of maintaining a pointer to the first node to properly manage the linked list structure. Additionally, there are recommendations to initialize variables before use and to utilize a debugger to step through the code for better insight into the problem. The conversation highlights common pitfalls in memory management and the value of debugging tools for new programmers.
Hallingrad
Messages
29
Reaction score
0
Hey guys,

I'm trying to implement a linked list in C, with each node carrying fields from a text file. The problem is that whatever was the last line read from the text file gets placed into every single node. Even more strange is that when I add an additional node outside of the while loop it doesn't replace all the previous nodes. Here's the loop that's causing me problems. Any help would be much appreciated!

Code:
while (!feof(file_ptrDR)) {
		
		newNode = (lnode *)malloc(sizeof(lnode));
                newNode = NULL;
	
		c = fgetc(file_ptrDR);
		while (c != ',') {
			NAME[i] = c;
			i++;
			c = fgetc(file_ptrDR);
			
		}
		
		i = 0;
		c = fgetc(file_ptrDR);
		newNode->name = NAME;
		while ((c = fgetc(file_ptrDR)) != ',') {
			PASSWORD[i] = c;
			i++;
		}
		i = 0;
		c = fgetc(file_ptrDR); //get past the space
		(newNode->password) = PASSWORD;
		c = fgetc(file_ptrDR);
		while (c != '\n' && c != EOF)  {
			TYPE[i] = c;
			c = fgetc(file_ptrDR);
			i++;
		}
		i = 0;
		newNode->type = TYPE;
		newNode->next = tempNode;
		tempNode = newNode;
}
linkedlist = tempNode;
 
Technology news on Phys.org
Code:
    newNode = (lnode *)malloc(sizeof(lnode));
    newNode = NULL;
Why are you zeroing out newNode just after allocating it? Perhaps you wanted this?
Code:
    newNode = (lnode *)malloc(sizeof(lnode));
    newNode->next = NULL;
Also you need to save the first instance of newnode in another variable so you have a pointer to the start of the linked list.
Code:
static lnode * firstNode = NULL;
...
    newNode = (lnode *)malloc(sizeof(lnode));
    if(firstNode == NULL)
        firstNode = newNode;
 
Last edited:
Jeff Reid said:
Code:
    newNode = (lnode *)malloc(sizeof(lnode));
    newNode = NULL;
Why are you zeroing out newNode just after allocating it? Perhaps you wanted this?
Code:
    newNode = (lnode *)malloc(sizeof(lnode));
    newNode->next = NULL;
Also you need to save the first instance of newnode in another variable so you have a pointer to the start of the linked list.
Code:
static lnode * firstNode = NULL;
...
    newNode = (lnode *)malloc(sizeof(lnode));
    if(firstNode == NULL)
        firstNode = newNode;

Making it null was just one of the various solutions I tried to rectify the problem. It didn't change the output when traveling through the list, but you're right, I should probably remove it.

As for the firstNode, where would I place it? The way I designed it, I'm adding new nodes to the head, so why would I need to save the very first newNode?
 
I didn't realize you wanted to create a LIFO (last in first out) linked list. I don't see how setting newNode = NULL isn't causing problems, because it will always throw away the pointer from malloc and instead set it to zero. I'm assuming that tempNode is initialized to NULL? Otherwise I don't see the problem. You might want to move the last i=0 to before the first part of the loop so it's clearly initialized to zero before the first time you parse "NAME".

Have you tried stepping through this code for a few loops with a debugger to see what's going on?
 
Jeff Reid said:
I didn't realize you wanted to create a LIFO (last in first out) linked list. I don't see how setting newNode = NULL isn't causing problems, because it will always throw away the pointer from malloc and instead set it to zero. I'm assuming that tempNode is initialized to NULL? Otherwise I don't see the problem. You might want to move the last i=0 to before the first part of the loop so it's clearly initialized to zero before the first time you parse "NAME".

Have you tried stepping through this code for a few loops with a debugger to see what's going on?

Making it not null didn't help.

It seems that any nodes made in the while loop get turned into what the last node was before the while loop exits, but if I manually add another node after that, it doesn't affect the previous nodes. Frankly I'm at a loss after hours of staring at the code.
 
So you did remove
Code:
     newNode = NULL;

If you run with the debugger, did you check to make sure newNode is being assigned a different address each time call malloc?
 
> Have you tried stepping through this code for a few loops with a debugger to see what's going on?

Why don't they teach new programmers about the debugger? It is so much easier to spot mistakes and it also teaches users a lot. It is the number one tool to use and no one seems to know about it.
 

Similar threads

Replies
1
Views
10K
Replies
7
Views
2K
Replies
3
Views
2K
Replies
6
Views
13K
Replies
6
Views
2K
Replies
1
Views
3K
Back
Top