Getting the Contents of a File and Putting it into a String in C-CodePudding

I am trying to make a function in C that takes the contents of a file and returns the contents as a string. I got it to works except for one odd detail. This is the current code:

char *getFileContents(const char *filePath) {
    if (filePath == NULL) return NULL;
    char buffer[1000];
    char character;
    int count = 0;
    FILE *f = fopen(filePath, "r");

    while (character != EOF) {
        count  ;
        character = fgetc(f);
        printf("%c\n", character);
    }
    count--;
    fclose(f);

    FILE *F = fopen(filePath, "r");
    char *str = (char*) malloc ( sizeof(char) * (count   1 ) );
    char *line = fgets(buffer, 1000, F);
    while (line != NULL) {
        strcat(str, line);
        line = fgets(buffer, 1000, F);
    }
    fclose(F);
    return str;
}

In the first while loop, I added a printf statement for error checking that I do not need anymore. The function works fine with the printf statement but whenever I comment it out or remove it I get a segmentation fault. I've used gdb to debug and try to find the issue. I can step through the whole function but the moment it reaches the return str at the end I get a segmentation fault. I'm not sure why I'm experiencing this problem.

CodePudding user response：

The commenting of a line causing a segmentation fault is the manifestation of Undefined Behaviour.

The initial comparison of character with EOF reads an uninitialized, garbage value.

char character;
/* ... */
while (character != EOF) {
/* ... */

The results of this program cannot be reasoned about after this occurs. A random SIGSEGV is par for the course.

Try changing your loop construct.

while (EOF != (character = fgetc(f))
    count  ;

Note, you should also check that the return value from fopen was not NULL, and react accordingly.

FILE *f = fopen(filePath, "r");

if (!f) {
    perror(filePath);
    return NULL;
}

CodePudding user response：

One problem is that you never initialize the memory buffer returned by malloc() -- you are apparently expecting it to be all-zeros (or at least start with a 0-byte) so that you can fill it in with calls to strcat(); but malloc() doesn't guarantee the contents of the memory it returns will be zeroed-out, so you are likely starting with some garbage-bytes already present in the str buffer, which means that as you add more bytes from the file, you will likely write past the end of the buffer and invoke undefined behavior (and likely crash).

The simple fix would be to do a str[0] = '\0'; after the call to malloc() (or if you're paranoid, you could do a full memset(str, 0, count 1);). Or alternatively you could allocate the memory with a call to calloc() rather than malloc(), as calloc() does guarantee that the returned buffer will be all zero-initialized bytes.