Program quits out after running?

Question

The program runs fine and displays the data I want but then crashes. I don't think it is a memory leak from the linked list because I've tried many ways to free it and none have worked.

#include "Buildings.h"

void displayBuilding(struct node *x){
    printf("Building Name: %s\n\tFloors: %d\n\tValue: %d\n\n", x->payload.name, x->payload.floors, x->payload.value);
    if(x->last!=1){
        displayBuilding(x->next);
    }
}

void insert(struct node *x, char str[50]){
    srand(t);
    int f = (rand() % 6)+1;
    int v = ((rand() % 20)+1)*f;
    //printf("Floors: %d and Value: %d\n", f, v);
    strcpy(x->payload.name, str);
    x->payload.floors = f;
    x->payload.value = v;
    srand(time(NULL));
    t+=f+v*(rand()%1000);
}

FILE* openData(FILE *f){
    f = fopen("Buildings-Data.txt", "r");
    if(f==NULL){
        printf("No File!");
        return 0;
    }
    return f;
}

void freeList(struct node* head)
{
   struct node* tmp;

   while (head != NULL)
    {
       tmp = head;
       head = head->next;
       free(tmp);
    }

}

int main(){
    FILE *f1;
    struct node *head;
    struct node *curr;
    head = malloc(sizeof(struct node));
    curr = malloc(sizeof(struct node));

    curr = head;
    int i;
    for(i=0;i<9;i++){
        curr->next = malloc(sizeof(struct node));
        curr = curr->next;
    }
    f1 = openData(f1);
    curr = head;
    int readNum;
    char trash;
    fscanf(f1, "%d", &readNum);
    fscanf(f1, "%c", &trash);
    for(i=0;i<readNum;i++){
        char str[50];
        fscanf(f1, "%s",str);
        insert(curr, str);
        curr->last = 0;
        if(readNum-i==1){
            curr->last = 1;
        }else{
            curr = curr->next;
        }
    }
    fscanf(f1, "%c", &trash);
    fclose(f1);
    curr = head;
    printf("\n");
    displayBuilding(curr);

    curr = head;
    freeList(head);

    return 0;
}

My header file:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <time.h>

struct building{
    char name[50];
    int floors;
    int value;  
};
typedef struct building place;

struct node{
    place payload;
    struct node *next;
    int last;
};

int t = 500;

Note that it is not a good idea to use global variables in general, and doubly so not a good idea to use single-letter names for global variables, and it is absolutely a bad idea to initialize a global variable in a header because it means only one source file in a program can use the header, and there's no point in creating a header if only one source file will ever use it. — Jonathan Leffler, May 06 '16 at 22:27
You have a memory leak because you first set `curr = malloc();`, and then immediately after that you do `curr = head`. So you've leaked the memory that you originally assigned to `curr`. But memory leaks don't cause crashes. — Barmar, May 06 '16 at 22:29
@MartinJames that worked and allowed the program to finish! Any thoughts as to why that was halting it? — George Tangen, May 06 '16 at 22:33
Not related to the problem, but you shouldn't call `srand()` every time you call `insert()`. Do it once at the beginning of the program. — Barmar, May 06 '16 at 22:34
@Barmar in relation to your comment about `srand()` I have that to allow the seed to change. When I had it seed just once in relation to time the random numbers were all the same. — George Tangen, May 06 '16 at 22:36
How many buildings are in the file? You're only allocating 9 items in your linked list. If there are more buildings, you'll go past the end of the list. — Barmar, May 06 '16 at 22:36
You invoke `srand()` twice each time you use `insert()` — that's bad. See [`srand()` — why call it only once?](http://stackoverflow.com/questions/7343833/srand-why-call-it-only-once/) for the details. It seems funny to preload 10 empty records into the list. The whole dynamic of reading records into the 'list' needs to be reviewed. And you probably need to check for more null pointers. — Jonathan Leffler, May 06 '16 at 22:36
@GeorgeTangen a bug in freeList() or an invalid 'head' argument. Debugger time! — Martin James, May 06 '16 at 22:37
@MartinJames `freeList()` looks pretty good to me. I suspect something else is causing undefined behavior before he calls it, and memory is getting corrupted. — Barmar, May 06 '16 at 22:38
@Barmar yes, good point, also very possible;) Given the number of bugs already commented on above, you could even say 'likely' :( — Martin James, May 06 '16 at 22:40
@Barmar I don't see anywhere that the head is ever set to NULL, 'head = malloc(sizeof(struct node))', so 'while (head != NULL)' seems like a bad plan:( — Martin James, May 06 '16 at 22:48
@MartinJames `head = head->next` is supposed to do that, except for the problem that I point out in my answer. — Barmar, May 06 '16 at 22:53
@Barmar I was getting there, albeit slowly. Anyway, have an upboat for firsties:) — Martin James, May 06 '16 at 22:58
@MartinJames If you intended that as a clue to the OP, you're living in a dreamland. — Barmar, May 06 '16 at 22:59

score 1 · Accepted Answer · answered May 06 '16 at 22:52

The problem is that when you create the linked list, you never set next to NULL in the last node of the list. You need to do that after the loop:

for(i=0;i<9;i++){
    curr->next = malloc(sizeof(struct node));
    curr = curr->next;
}
curr->next = NULL;

Because of this, the while (head != NULL) loop in freeList() never stops, and eventually tries to free the uninitialized pointer in the last node of the list.

Program quits out after running?

1 Answers1