Search code examples
cstringmallocfgetsrealloc

Program breaks after a couple hundred lines have been read


I made a scanning function which essentially just scans in the lines of a file into a single char * called buffer. However, after a couple hundred lines are read, the program just stops working. I just get a program has stopped working pop up window. Assuming I did something wrong with memory allocation, but I am unsure what.

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

char *scan_file(FILE *fp);

#define MAX_LINE                200


int main(void) {
    FILE *fp = fopen("test.txt", "r");
    char *contents = scan_file(fp);
    printf("%s\n", contents);
    return 0;
}
// Scan in file into a buffer. Returns a malloc-ed string
char *scan_file(FILE *fp) {

    int buf_len = 1;
    int contents_len = buf_len;

    char *buffer = malloc(sizeof(char) * (MAX_LINE + 1));
    char *contents = malloc(sizeof(char) * (buf_len + 1));
    strcpy(contents, "\0");

    while (fgets(buffer, MAX_LINE, fp) != NULL) {
        buf_len = strlen(buffer);
        contents_len += buf_len;
        realloc(contents ,contents_len);
        strcat(contents, buffer);
        strcpy(buffer, "\0");
    }

    free(buffer);
    return contents;
}

Solution

  • Code fails to use the return value form realloc()

    Allocation sizes off by 1.

    Repeated strcat() make for a slow (n*n) solution.

    Consider using size_t for array sizes vs. int.

    Rather than call the variable ...len, consider ...size to acknowledge the presence of the final null character in a string.

    char *scan_file(FILE *fp) {
        // int buf_len = 1;
        size_t buf_size = 1;
    
        // int contents_len = buf_len;
        size_t contents_size = buf_size;
    
        // char *buffer = malloc(sizeof(char) * (MAX_LINE + 1));
        // fgets(..., MAX_LINE, ...) will only read up to MAX_LINE - 1 characters.
        char *buffer = malloc(MAX_LINE);
    
        char *contents = malloc(buf_size + 1u);
        if (buffer == NULL || contents == NULL) {
          fprintf(stderr, "Out of memory\n");
          return EXIT_FAILURE;
        }
    
        // strcpy(contents, "\0");
        contents[0] = '\0';
    
        while (fgets(buffer, MAX_LINE, fp) != NULL) {
            // buf_len = strlen(buffer);
            buf_size = strlen(buffer) + 1u;
    
            // contents_len += buf_len;
    
            // realloc(contents ,contents_len);
            void *p = realloc(contents ,contents_size + buf_size);
            if (p == NULL) {
              fprintf(stderr, "Out of memory\n");
              return EXIT_FAILURE;
            }
            contents = p;
    
            // strcat(contents, buffer);
            strcpy(contents + contents_size, buffer);
    
            // now add
            contents_size += buf_size;
    
            // Code here not truly needed, yet helps in debugging.
            // strcpy(buffer, "\0");
            buffer[0] = '\0';
        }
    
        free(buffer);
        return contents;
    }