Reading from file fails

Question

I am reading a string from a file. After like the second or third time the function gets executed, one or more random characters become appended to the buffer string and I have no idea why that happens.

Here's the piece of code:

scorefile = fopen("highscore.dat", "rb");

if (scorefile)
{
    fseek(scorefile, 0, SEEK_END);
    length = ftell(scorefile);
    fseek(scorefile, 0, SEEK_SET);
    buffer = malloc(length);
    if (buffer)
    {
        fread(buffer, 1, length, scorefile);
    }
    fclose(scorefile);
}

Am I doing something wrong here?

How about doing some debugging and/or implementing proper [error handling](http://stackoverflow.com/questions/21267716/error-handling-in-file-opening)? — Uwe Keim, Sep 04 '16 at 13:59
My crystal ball thinks that you're treating `buffer` as a zero-terminated string, which it isn't (unless your file has a zero at the end). — molbdnilo, Sep 04 '16 at 14:01
@Uwe Klein: I am doing error handling around this function. While debugging, I can only see how the buffer is empty before fread gets called and after that call, the content from the file is in but also unwanted characters. How can I debug it deeper? — , Sep 04 '16 at 14:01
then the problem happens when you write the file. What it is supposed to contain? ascii? binary? — Jean-François Fabre, Sep 04 '16 at 14:02
@Jean-FrançoisFabre: Yes - I can see the faulty string in the debugger already, though. The actual content of the file is okay so it must happen when reading from the file. — , Sep 04 '16 at 14:04
@molbdnilo it is not what wh think. but OP is not clear at all. good luck y'all. — Jean-François Fabre, Sep 04 '16 at 14:08
@Kai The debugger thinks that `buffer`is a zero-terminated string because it's a `char*`. The "appended characters" are what's lying around in memory past the last `char`you read.. — molbdnilo, Sep 04 '16 at 14:10
@molbdnilo Sounds plausible. So I need to store an \0 into the file the program reads from? — , Sep 04 '16 at 14:17
@Kai Only if it really is supposed to be a zero-terminated string. Otherwise, there's no problem with what you have now. — molbdnilo, Sep 04 '16 at 14:22
@molbdnilo It isn't. I'm trying to fix this problem for days now. Can't get beyond what is happening. Sometimes it appends the path to other programs like Internet Explorer, sometimes it appends dots, sometimes just strange random chars..... — , Sep 04 '16 at 14:24
@Kai From your description it sounds like an artefact of how the debugger interprets things. Are you having any actual problems besides that cosmetic one? — molbdnilo, Sep 04 '16 at 14:45
@molbdnilo Well either the highscore has entries with dots and 21568465 points instead of like 120 and sometimes the program crashes because it seems like those faulty characters can't be understood from the program. — , Sep 04 '16 at 14:52
"after that call, the content from the file is in but also unwanted characters." - you *know* this *how*? This code has no return value checking *at all*. We're led to assume the file is indeed open, that both `fseek` invokes *worked*, that `length` is correct, that `fread` actually returned the same `length` value for the number of bytes requested as the number actually *read*, *and* that however, this data is interpreted as "correct" expects exactly what you gave it. It would seem there is room for some deeper error checking *and* value verification. — WhozCraig, Sep 04 '16 at 15:50

cdlane · Answer 1 · 2016-09-04T23:57:55.633

1

Let's spell it all out and go slightly more robust:

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

char *loadScoreFile(const char *filename)
{
    char *buffer = NULL;

    FILE *scorefile = fopen(filename, "r");

    if (scorefile != NULL)
    {
        (void) fseek(scorefile, 0, SEEK_END);
        int length = ftell(scorefile);

        (void) fseek(scorefile, 0, SEEK_SET);

        buffer = malloc(length + 1);

        if (buffer != NULL)
        {
            assert(length == fread(buffer, 1, length, scorefile));

            buffer[length] = '\0';
        }
        (void) fclose(scorefile);
    }

    return buffer;
}

int main()
{
    for (int i = 0; i < 10; i++)
    {
        char *pointer = loadScoreFile("highscore.dat");

        if (pointer != NULL)
        {
            printf("%s", pointer);
            free(pointer);
        }
    }

    return 0;
}

edited Sep 04 '16 at 23:57

answered Sep 04 '16 at 17:10

cdlane

40,441
5
32
81

The content of the scorefile is "10-Test;" and the result of the print is: http://prntscr.com/ce6lmt – Sep 04 '16 at 22:48
@Kai, that's from running the above code unmodified? I ran it with the contents you mentioned and just keep getting "10-Test;" over and over. You might try changing the open mode from "rb" to simply "r" since we're just reading text and see if that makes a difference. Otherwise, I don't know. You might search for SO for "fopen Windows" to see if there are known glitches. – cdlane Sep 05 '16 at 00:08
could it be that maybe something else in my program could cause this? Like a bug somewhere else and I only see the cause here. After all I am really just trying to read text from a file - no idea why it seemes so cursed... – Sep 05 '16 at 00:58

score 0 · Answer 2 · answered Sep 04 '16 at 14:16

0

if you use buffer = malloc(length);, and then read length bytes into it, it will be one byte too short. Char arrays in C are zero-terminated, so they need an extra byte to but that zero. buffer = malloc(length+1); will fix this.

answered Sep 04 '16 at 14:16

Aganju

6,295
1
12
23

It didn't, unfortunately. – Sep 04 '16 at 14:18

Reading from file fails

2 Answers2