fscanf text file with dynamic allocated memorry

Question

I have to do a function that reads a text file with characters. It is obligatory to use malloc and realloc. I made this code, whitout errors, but when I try to read file, I get runtime error. And I can't understand where is the problem in this code.

void openFile(FILE** file,char* filename)
{
int SIZE=10;
char* data,*data2;
int n = 0;
char c;
printf("filename:");
scanf("%s",&*filename);
if (!((*file) = fopen(filename, "r")))
    perror("Error:");
else
{   
    if ((data = (char*)malloc(SIZE * sizeof(char))) == NULL)
    {
        perror("malloc:");
    }   
    while (fscanf((*file),"%s",c) != EOF)
    {
        if (n < SIZE)
        {
            data[n++] = c;
        }
        else
        {
            if ((data2 = (char*)realloc(data, SIZE * sizeof(char))) != NULL)
            {
                data = data2;
                data[n++] = c;  
            }
            else
            {
                free(data);
            }
        }

    }
}
}

Note that [accepting answers](https://meta.stackexchange.com/questions/5234/how-does-accepting-an-answer-work) is one of the features of the site, it's important to reward the good answers and to make future users know which is the answer that solves the secific problem proposed by the question, doing this also increases the chances of getting good answers. I'm telling you this because the first question you made here has some good answers and you didn't accept any of them, then the next two have no answers. — anastaciu, Apr 25 '20 at 00:25

anastaciu · Accepted Answer · 2020-04-26T00:29:41.013

There are some issues with your code, almost none of them fatal, deppending on what you pass to the function.

The catastrophic failure is probably because in your fscanf function you using "%s" specifier with a char variable, the correct specifier is %c, and you need to pass the address of the variable &c.
You should address scanf("%s", &*filename);, there is the danger of the data exceeding the storage capacity of the memory buffer, you should allways define a max size not larger than the capacity of the buffer, you can use "%99s" specifier with scanf for a memory buffer of 100 characters, or better yet, using fgets:

   fgets(filename, sizeof(filename), stdin);
   filename[strcspn(filename, "\n")] = '\0'; //to remove newline character

The way you are using the file pointer makes me suspect that you wouldn't need to pass it as an argument and much less as a double pointer, this would be useful if you need to keep the pointer to the file stream outside the function, if that's the case, you can leave it as is, otherwise you can use a local variable and close the stream when you are finished.

Here is he code with some corrections as mentioned, and some other minor ones:

void openFile(char *filename)
{
    int SIZE = 10;
    char *data, *data2;
    int n = 0;
    char c;
    FILE *file; //local variable
    printf("filename:");
    scanf("%99s", filename);// &* is pointless, using %99s, assuming filename[100]
    if (!(file = fopen(filename, "r")))
        perror("Error:");
    else
    {
        if ((data = malloc(SIZE)) == NULL) //char size is always 1 byte, no cast needed, include stdlib.h
        {
            perror("malloc:");
        }
        while (fscanf(file, "%c", &c) != EOF) //specifier for char is %c, furthermore you need & operator
        {                                     //probably the source of your problems
            if (n < SIZE)
            {
                data[n++] = c;
            }
            else
            {
                if ((data2 = realloc(data, SIZE)) != NULL) // again no cast, no sizeof(char)
                {
                    data = data2;
                    data[n++] = c;
                }
                else
                {
                    free(data);
                }
            }
        }
        fclose(file); //close file stream
    }
}

Note that the only catastrophic problems are the ones with fscanf, you can fix those only and the code will likely work, I'd still advise the other fixes.

Was the intention, perhaps, to pass a `FILE **file` as an argument so that `*file = fopen(filename, "r"); if (*file == NULL) { perror(filename); }` can be used to return the opened file stream (given that it isn't closed before return — otherwise, you're leaking file streams). Also, there's no point in passing the file pointer if the code simply overwrites it with a local value. It should be a local variable, not an argument. — Jonathan Leffler, Apr 25 '20 at 03:10
`scanf("%s", filename);` is as good as [`gets`](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used)`. True, not OP's key issue, yet dubious improvement. — chux - Reinstate Monica, Apr 25 '20 at 08:53
@chux-ReinstateMonica, yes, you're right, I edited to address this issue. — anastaciu, Apr 25 '20 at 12:09

fscanf text file with dynamic allocated memorry

1 Answers1