Matthew Caixeiro
Matthew Caixeiro

Reputation: 23

fclose() crashing c program

I have a relatively simple program that keeps crashing after reading integers from a file. It crashes upon execution of the fclose line. I have localized the error to be in this function.

// Read array from text file
int * fileRead() {
    FILE *file;
    file = fopen("test.txt", "r");

    // Check if the file exists
    if(file == NULL){
        printf("There was a problem opening the file");
        exit(1);
    }

    // Count number of lines in file
    int count = 0;
    char c;
    for (c = getc(file); c != EOF; c = getc(file)){
        if (c == '\n') { 
            count = count + 1;
        }
    }

    // Reset to top of file
    int t = fseek(file, 0, SEEK_SET);

    // Read each line and save it to temp
    int *temp = malloc(sizeof(int)*count);
    int num, i;
    for (i = 0; i<=count; i++){
        fscanf(file, "%d\n", &temp[i]);
        printf("%d\n", temp[i]);
    }

    fclose(file);
    printf("Hello World\n");
    return temp;
}

The Hello World is to prove to myself it crashes at fclose exactly. The function reads ints from a file which contains only ints on separate lines (of unknown quantity) and saves them to an array and then returns that array. Thank you in advance for the help, this is my first time using c so I am not sure where to start looking for the bug.

Upvotes: 2

Views: 2478

Answers (2)

chqrlie
chqrlie

Reputation: 144520

There are multiple problems in your code:

  • fgetc(fp) returns an int value that can have all the values of type unsigned char and the special value EOF. You must store this into a variable of type int for the end of file test to be reliable.

  • in the loop for (i = 0; i<=count; i++){ you cause a buffer overflow as the maximum allowed index into the allocated block is count-1. This is definitely causing undefined behavior and may well explain the observed behavior.

    Use this instead:

      for (i = 0; i < count; i++) ...
    
  • you must pass the length of the allocated array back to the caller.

  • counting the number of lines does not accurately determine the number of entries in the file. It would be better to parse the file twice or to parse it just once and reallocate the array dynamically. The latter approach is necessary for non seekable input streams such as terminals and pipes.

Here is a modified version of your code:

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

int *fileRead(int *countp) {

    // Check if the file exists
    FILE *file = fopen("PATH.txt", "r");
    if (file == NULL) {
        fprintf(stderr, "Cannot open input file PATH.txt: %s\n",
                strerror(errno));
        exit(1);
    }

    // Count number of lines in file
    int num, count = 0;
    while (fscanf(file, "%d", &num) == 1) {
        count++;
    }

    // Reset to top of file
    fseek(file, 0, SEEK_SET);

    // Read each line and save it to temp
    int *temp = calloc(sizeof(int), count);
    if (temp != NULL) {
        int i;    
        for (i = 0; i < count; i++) {
            if (fscanf(file, "%d", &temp[i]) != 1) {
                fprintf(stderr, "error reading element number %d\n", i);
                break;
            }
        }
        *countp = i;  // number of entries successfully converted
    }
    fclose(file);
    return temp;
}

int main(void) {
    int count;
    int *t = fileRead(&count);
    if (t != NULL) {
        for (int i = 0; i < count; i++) {
            printf("%d\n", t[i]);
        }
        free(t);
    }
    return 0;
}

Upvotes: 1

Tony Tannous
Tony Tannous

Reputation: 14856

NOTES:

Indices in c starts from 0. So if you want to save count integers, you have to iterate till count-1.

i.e.

  • i < count
  • i <= count-1

Your reading is wrong, as you assume your integers are of one digit.

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

int * fileRead() {
    FILE *file;
    file = fopen("PATH.txt", "r");

    // Check if the file exists
    if(file == NULL){
        printf("There was a problem opening the file");
        exit(1);
    }

    // Count number of lines in file
    int count = 0;
    char pc = '\n';
    char c;
    while (c = fgetc(file), c != EOF)
    {
        if (c == '\n'  &&  pc != '\n')
            count++;
        pc = c;
    }

      // Reset to top of file
    fseek(file, 0, SEEK_SET);

    // Read each line and save it to temp
    int *temp = malloc(sizeof(int)*count);
    int num, i;

    for (i=0; i<count; i++)
    {
        fscanf (file, "%d", &temp[i]);
    }

    fclose(file);
    return temp;
}


int main()
{
    int *t = fileRead();
    printf("%d\n", t[0]);
    printf("%d\n", t[1]);
}

FILE:

452
55

OUTPUT:

542
55

To sum it up:

fclose() crashing my program.

No. It wasn't fclose it was you trying to access memory not allocated.

Upvotes: 1

Related Questions