Am I doing something wrong in my file-reader function?-CodePudding

I'm a beginner to C and wanted to code a simple function that reads the content of file and returns it as a string, as an exercise.

Here is my solution which I think works, but is there any obvious bad practices or unoptimal code here ? For example, I manually added a \0 at the end of the string, but I don't know if it is really necessary...

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

char *readFile(char *path)
{
    //open file
    FILE *file = fopen(path, "r");
    //if broken
    if (file == NULL)
    {
        printf("Erreur");
        return NULL;
    }

    //return variable
    char *result;

    //length of the file
    int len;
    fseek(file, 0, SEEK_END);
    len = ftell(file);
    fseek(file, 0, SEEK_SET);

    //initialising return variable
    result = (char*) malloc(sizeof(char) * (len   1));

    int c;
    int i = 0;
    while (feof(file) == 0)
    {
        c = fgetc(file);
        if (c != EOF)
        {
            printf("x -> %c\n", c, c);
            *(result   i) = c;
            i  ;
        }
    }
    *(result   i) = '\0';
    printf("len : %i\n", len);
    fclose(file);

    return result;
}

CodePudding user response：

Since you have already got the length of the file to be read, you could also read them at once instead char-by-char.

Another implmentation of your function, for example:

char *readFile(char *path)
{
    //open file
    FILE *file = fopen(path, "r");
    //if broken
    if (file == NULL)
    {
        printf("Erreur");
        return NULL;
    }

    //return variable
    char *result;

    //length of the file
    int len;
    fseek(file, 0, SEEK_END);
    len = ftell(file);
    fseek(file, 0, SEEK_SET);

    //initialising return variable
    result = (char*) malloc(sizeof(char) * (len   1));

    size_t i = fread(result, sizeof(char), len, file);

    *(result   i) = '\0';
    printf("len : %i\n", len);
    fclose(file);

    return result;
}

CodePudding user response：

I'd replace this:

    int c;
    int i = 0;
    while (feof(file) == 0)
    {
        c = fgetc(file);
        if (c != EOF)
        {
            printf("x -> %c\n", c, c);
            *(result   i) = c;
            i  ;
        }
    }

with this:

    fread(file, 1, len, result);

It's much shorter
It's correct
It's certainly faster

There is still room for improvement though, for example you could add error handling, fread can fail.