Can't read file in C using fread()

Question

Program asks for input and stores it in a variable, then confirms the operation printing the content of the file. Or at least it had to, when the program ends it doesn't print the file content, I can't seem to find an answer, I've been looking in the docs but can't really figure it out.

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

int main(void)
{
    FILE * file1 = fopen(".out", "w+");

    char *s = malloc(513);
    fgets(s, 513, stdin);
    
    if (fprintf(file1, "%s", s) < 0)
    {
        printf("Something failed while writing to the file\n");
        return 1;
    }
    else
    {
        char *t = malloc(513);
        fread(t, sizeof(char), 1, file1);
        printf("Success! Input was: %s \n", t);
        return 0;
    }
}

P.S: Very new to C, though it may seem obvious for you I have no clue whatsoever.

`fread(t, sizeof(char), 1, file1);` How many character do you expect that to read? Furthermore, after the `fprintf` the file pointer is at the end of the file. Need to `rewind` it before trying to `fread`. — kaylum, Jul 11 '22 at 01:23
Mario Olcina. `fread(t, sizeof(char), 1, file1);` attempts to read one character. If that your intent? — chux - Reinstate Monica, Jul 11 '22 at 03:12
Mario Olcina, `printf("Success! Input was: %s \n", t);` is for printing a _string_. `t` does not point to a _string_ as it lacks a _null character_. Result: undefined behavior. — chux - Reinstate Monica, Jul 11 '22 at 03:14
Is there some reason for using `513`? How did you determine buffer would need to be that size? — ryyker, Jul 11 '22 at 12:52
@ryyker Yes, I'm using 513 to accomodate a 512 byte long string + the NUL character. — Mario Olcina, Jul 11 '22 at 13:20
My point was that the program itself does not determine how many byes it needs to read from the file before reading it. You could have, for example [used fseek()](https://www.geeksforgeeks.org/c-program-find-size-file/) before the call to `fopen()`. As is, `513` is just a [magic number](https://stackoverflow.com/questions/47882/what-is-a-magic-number-and-why-is-it-bad), that logically appears out of nowhere. — ryyker, Jul 11 '22 at 13:25

score 0 · Accepted Answer · answered Jul 11 '22 at 04:11

0

There are 2 issues here, 1 - you wrote to the file handler and you are trying to read from that point onwards - you didnt rewind the file pointer! 2 - you are just reading 1 character and not the amount you wrote to it!

#include <string.h>

...

        int n = strlen(s);
        rewind(file1);   // rewind before read 
        fread(t, sizeof(char), n, file1);   // read as much as you wrote

answered Jul 11 '22 at 04:11

Ani

1,448
1
16
38

Thank you! I see now! I didn't know about rewind. – Mario Olcina Jul 11 '22 at 13:22
In OP, there is the statement phrase: `char *t = malloc(513); fread(t, sizeof(char), 1, file1);`. This leaves `t` uninitialized prior to writing to it, potentially resulting in UB. Subsequently in your example snippet: fread(t, sizeof(char), n, file1);` places exactly `strlen(s)` characters into `t`. It would have been more idiomatic to use the pointer `s` (the intended buffer) to receive the read, and to add another byte to the length parameter to provide for the null terminator for the string buffer.: `fread(s, 1, n+1, file1);` (Note `sizeof char` is always `1` by definition) – ryyker Jul 11 '22 at 19:26
@ryyker ok, I wanted to just highlight the changes to get the code working. Agree, that there has to be a lot more error handling. – Ani Jul 12 '22 at 10:12

score 0 · Answer 2 · answered Jul 11 '22 at 09:55

Some problems in your code:

You are not checking the return value of fopen(), malloc(), fgets() and fread().
You are writing one character to the output stream, without rewinding it.

Here's how your code should look like:

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

int main(void)
{
    FILE * file1 = fopen(".out", "w+");
    if (!file1) {
        printf("Could not open file.\n");
        return 1;
    }

    const size_t n = 513; // Use constants, not litterals.
    char *s = malloc(sizeof(char) * n);
    if (!s) {
        printf("Internal error.\n");
        fclose(file1);
        return 1;
    }
    
    if (!fgets(s, n, stdin)) {
        printf("Input failed.\n");
        fclose(file1);
        return 1;
    }
    
    if (fprintf(file1, "%s", s) < 0) {
        printf("Something failed while writing to the file\n");
        fclose(file1);
        return 1;
    }
    
    char *t = malloc(sizeof(char) * n);
    if (!t) {
        printf("Internal error.\n");
        fclose(file1);
        return 1;
    }
    
    rewind(file1);
    
    int ret = fread(t, sizeof(char), n, file1); // Read n characters, not 1.
    if (ret != strlen(s)) {
        if (feof(file1)) {
            printf("Error reading .out: unexpected end of file.\n");
        } else if (ferror(file1)) {
            perror("Error reading .out");
        }
        fclose(file1);
        return 1;
    }
    
    printf("Success! Input was: %s \n", t);
}

`fread(t, sizeof(char), n, file1);` writes to an uninitialized buffer, filled with who knows what, and doesn't provide room for the null terminator. UB when buffer is used in any subsequent call to a string function. — ryyker, Jul 11 '22 at 19:32
`malloc()` does not initialize memory, it simply allocates it. For applications such as this one it can be important to follow that call by initializing the memory, depending on how it is subsequently used, perhaps with a call to `memset(t, 0, n);` (note that `calloc()` initializes the memory it allocates.) — ryyker, Jul 12 '22 at 12:15

Can't read file in C using fread()

2 Answers2