0

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;
}
aqamn
  • 13
  • 1
  • 3
  • 4
    There's a lot of other problems, but basically `s = (char*) malloc(sizeof(char));` allocates just a single byte. – Ken Y-N Feb 13 '18 at 01:38
  • Start with a small buffer, like `s = malloc(1024)`, and then use `realloc` to make it bigger, if needed. – user3386109 Feb 13 '18 at 01:40
  • You realize `malloc(sizeof(char))` is `malloc(1)`, right? Presumeably you intend your input lines to have more than 1 byte? – Lee Daniel Crocker Feb 13 '18 at 01:42
  • There are more lines, I dont know exactly how much. – aqamn Feb 13 '18 at 01:47
  • regarding: `s = (char*) malloc(sizeof(char));` this only allocates a single byte and never uses `realloc()` to increase the allocation, to any index value above 0 is undefined behavior and can (as you have seen) result in a seg fault event. – user3629249 Feb 13 '18 at 04:26

3 Answers3

3

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;
}
Pablo
  • 13,271
  • 4
  • 39
  • 59
  • Thank you, you helped a lot! If I don't know the size of the txt. file, what should I write at malloc? (I will have an exam, and the size will not be specified I guess) What do you suggest? – aqamn Feb 13 '18 at 02:14
  • @aqamn I've made an edit of my answer addressing your comment. – Pablo Feb 13 '18 at 02:40
0

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.

sth
  • 222,467
  • 53
  • 283
  • 367
  • Thank you for helping, I allocated more space for s. But I still have a problem at printing, the first few characters are incorrect. – aqamn Feb 13 '18 at 01:53
0

Calculate size of the file with the help of fseek.

fseek(fp, 0L, SEEK_END); sz = ftell(fp);

You can then seek back,to the begining of file

fseek(fp, 0L, SEEK_SET); Allocate that much memory through malloc. S= (char*)malloc((sizeof(char)×sz)+1) Now you can use this memory to copy bytes in while lopp

Rohit
  • 142
  • 8