My code crashes at long texts? Reading from file character by character into a dynamic char array in C

I want to read a whole text from txt file, then print it. It should be with dynamic memory managment and character by character. My code works good with short texts (max. 40 characters), but crashes when I read more characters.

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

int main()
{
   char *s;
   int n, i;
   s = (char*) malloc(sizeof(char));
   n=0;
   FILE *f=fopen("input.txt","r");
   while (fscanf(f, "%c", &s[n]) != EOF)
        n++;
   fclose(f);
   free(s);
   for (i=0;i<n;i++)
   printf("%c",s[i]);
   return 0;
}

2 answers

  • answered 2018-02-13 01:35 sth

    You are only allocating space for one single char (sizeof(char)):

    s = (char*) malloc(sizeof(char));
    

    If you want to store more characters you have to allocate more space, for example:

    s = (char*) malloc(sizeof(char) * 100);
    

    To read the whole file you best first find out how large the file is, so that you know how much space you need to allocate for s.

  • answered 2018-02-13 01:35 Pablo

    Please don't cast malloc.

    You allocating the wrong number of bytes, sizeof(char) returns you the size of a single char, so you are allocating one byte only. That's not enough to hold a string.

    You should use malloc like this:

    int *a = malloc(100 * sizeof *a);
    if(a == NULL)
    {
        // error handling
        // do not continue
    }
    

    Using sizeof *a is better than sizeof(int), because it will always return the correct amount of bytes. sizeof(int) will do that as well, but the problem here is the human factor, it's easy to make a mistake and write sizeof(*int) instead. There are thousands of questions here with this problem.

    Note that a char is defined to have the size of 1, that's why when you are allocating memory for strings or char arrays, people usually don't write the * sizeof *a:

    char *s = malloc(100);
    // instead of
    char *s = malloc(100 * sizeof *s);
    

    would be just fine. But again, this is only the case for char. For other types you need to use sizeof operator.

    You should always check the return value of malloc, because if it returns NULL, you cannot access that memory.

    while (fscanf(f, "%c", &s[n]) != EOF)
        n++;
    

    If you for example allocated 100 spaces, you have to check that you haven't reached the limit. Otherwise you will overflow s:

    char *s = malloc(100);
    if(s == NULL)
    {
        fprintf(stderr, "not enough memory\n");
        return 1;
    }
    
    int n = 0;
    
    while ((fscanf(f, "%c", &s[n]) != EOF) && n < 100)
        n++;
    

    In this case you are not using the allocated memory to store a string, so it's fine that it doesn't have the '\0'-terminating byte. However, if you want to have a string, you need to write one:

    while ((fscanf(f, "%c", &s[n]) != EOF) && n < 99)
        n++;
    
    s[n] = '\0';
    

    Also you are doing this

    free(s);
    for (i=0;i<n;i++)
        printf("%c",s[i]);
    

    You are freeing the memory and then trying to access it. You have to do the other way round, access then free.

    Correct way:

    for (i=0;i<n;i++)
        printf("%c",s[i]);
    free(s);
    

    EDIT

    If you want the contents of a whole file in a single string, then you have 2 options:

    1. Calculate the length of the file beforehand and then use allocate the correct amount of data
    2. Read one fixed size chunk of bytes at a time and resize the memory every time you read a new chunk.

    The first one is easy, the second one is a little bit more complicated because you have to read the contents, look how much you've read, resize the memory with realloc, check that the resizing was successful. This is something you could do later when you have for knowledge in simple memory managment.

    I'll show you the first one, because it's much easier. The function fseek allows you to advance your file pointer to the end of the file, with the function ftell you can get the size of the file and with rewind rewind the file pointer and set it to the beginning:

    #include <stdio.h>
    #include <stdlib.h>
    
    int main(int argc, char **argv)
    {
        if(argc != 2)
        {
            fprintf(stderr, "usage: %s file\n", argv[0]);
            return 0;
        }
    
        FILE *fp = fopen(argv[1], "r");
    
        if(fp == NULL)
        {
            fprintf(stderr, "Could not open %s for reading.\n", argv[1]);
            return 1;
        }
    
        // calculating the size
    
        // setting file pointer to the end of the file
        if(fseek(fp, 0L, SEEK_END) < 0)
        {
            fprintf(stderr, "Could not set the file pointer to the end\n");
            fclose(fp);
            return 1;
        }
    
        // getting the size
        long size = ftell(fp);
    
        if(size < 0)
        {
            fprintf(stderr, "Could not calculate the size\n");
            fclose(fp);
            return 1;
        }
    
        printf("file size of %s: %ld\n", argv[1], size);
    
        // rewinding the file pointer to the beginning of the file
        rewind(fp);
    
        char *s = malloc(size + 1); // +1 for the 0-terminating byte
    
        if(s == NULL)
        {
            fprintf(stderr, "not enough memory\n");
            fclose(fp);
            return 1;
        }
    
        int n = 0;
    
        // here the check && n < size is not needed
        // you allocated enough memory already
        while(fscanf(fp, "%c", &s[n]) != EOF)
            n++;
    
        s[n] = '\0'; // writing the 0-terminating byte
    
        fclose(fp);
    
        printf("Contents of file %s\n\n", argv[1]);
    
        for(int i=0; i<n; i++)
            printf("%c",s[i]);
    
        free(s);
        return 0;
    }