Linked List - program crashes while adding nodes

Question

My code compiles correctly, but after 4 loops of my insertLast function, the program crashes. Can someone help me understand why?

I posted a similar question earlier which helped me identify other problems. I've rewritten the function but I still have the same problem. My code below:

#include <stdio.h>
#include <stdlib.h>
#include "LinkedList.h"
int main (int argc, char* argv[])
{
 int ii;
 {
 FILE* f; /*open file for reading to get ints*/
 f = fopen(argv[1], "r");
 if(f==NULL) 
 {
 printf("Error: could not open file");
 return 0;
 }
 LinkedList* canQueue=createList();
 for(ii = 0; ii < 10; ii++)
 {
 TinCan* tempCan= (TinCan*) malloc(sizeof(TinCan));
 fscanf(f, " WGT_%d", &tempCan[ii].weight);
 insertLast(canQueue, tempCan); /*Inserts the new can into linked list*/
 }
 testLinkedList(canQueue);
 }
 return 0;
}

LinkedList.h

typedef struct TinCan
 {
 int weight;
 } TinCan;
typedef struct Node
 {
 TinCan* data;
 struct Node *next;
 } Node;
typedef struct LinkedList
 {
 Node *head;
 } LinkedList;
void insertLast(LinkedList* list, TinCan *newData);
LinkedList* createList();
void testLinkedList(LinkedList* list);

LinkedList.c

#include <stdio.h>
#include <stdlib.h>
#include "LinkedList.h"
LinkedList* createList() /*creates empty linked list*/
 {
 LinkedList* myList;
 myList = (LinkedList*)malloc(sizeof(LinkedList));
 myList->head = NULL;
 return myList;
 }
void insertLast(LinkedList* list, TinCan *newData)
 {
 Node* newNode = (Node*)malloc(sizeof(Node));
 newNode->data = newData;
 newNode->next = NULL;
 if(list->head==NULL)
 {
 Node* current = (Node*)malloc(sizeof(Node));
 list->head=newNode;
 current=newNode;
 }
 else
 {
 Node* temp = (Node*)malloc(sizeof(Node));
 temp = list->head;
 while(temp->next!=NULL)
 {
 temp = temp->next;
 }
 temp->next = newNode;
 }
 printf("Looped\n");
 }
void testLinkedList(LinkedList* list)
 {
 Node* current;
 current = list->head;
 while(current != NULL)
 {
 printf("Weight = %d\n", current->data->weight);
 current = current->next;
 }
 }

• DarenW: Looking only quickly over your code - when you insertLast() to an empty list, are you malloc'ing two Node structs? It is not obvious why.

Answer 1

These lines can be removed:

Node* current = (Node*)malloc(sizeof(Node));
current=newNode;

This line doesn't need an allocation of memory:

Node* temp = (Node*)malloc(sizeof(Node));

I bet you are actually breaking on this line though:

fscanf(f, " WGT_%d", &tempCan[ii].weight);

tempCan isn't an array, I'm not 100% sure what the &tempCan[ii] will do, but I suspect you're accessing memory around your tempCan pointer location and that it's only working for 4 because that's the size of something.

• Dawson: You are right, I can't believe I missed that. Changed it to &tempCan->weight and it works. Thanks!
• Matthias: @Dawson: better you remove the suggested lines too. If not, you will run into a memory leak.
• Dawson: How would I declare temp if I remove the malloc here Node* temp = (Node*)malloc(sizeof(Node))
• Matthias: Simply declare it, but do not assign it.
• Jim Balter: @Dawson Node* temp = list->head;

Answer 2

In for loop,

fscanf(f, " WGT_%d", &tempCan[ii].weight);

instead do

fscanf(f, " WGT_%d", &tempCan->weight);

tempCan has allocated for only 1 element. As your loop counter increments, you as accessing invalid location.