Reading from file with SDL_RWread appends a previous string

SDL Development
1

I’m reading files in a C99 program that will likely target many platforms that SDL supports, so I want to use SDL_RWops. I’m testing on a macOS machine.

Here’s my file loading code:

void loadFile(const char fileName[]) {
    char filePath[SDL_strlen(mainDirectory) + SDL_strlen(fileName) + 2];
    SDL_snprintf(filePath, sizeof(filePath), "%s/%s", mainDirectory, fileName);

    SDL_RWops* file = SDL_RWFromFile(filePath, "r");
    if (file == NULL) {
        SDL_Log("%s\n", "File could not be found in this directory.");
        return;
    }

    const Sint64 fileSize = SDL_RWsize(file);
    char fileContents[fileSize];

    SDL_ClearError();
    if (SDL_RWread(file, &fileContents, sizeof(char), fileSize) == 0) {
        const char* err = SDL_GetError();
        if (strcmp(err, "") == 0) {
            SDL_Log("%s\n", "Success, end of file.");
        } else {
            SDL_Log("%s\n", err);
        }
    }

    SDL_RWclose(file);
    SDL_Log("File contents: %s\n", fileContents);
}

For testing, I have a file that contains this content:

stuff and things

“mainDirectory” is also defined like this:

// in the header
static char* mainDirectory;

// later on, in a function in the associated source file
if (argc <= 1) {
    mainDirectory = SDL_GetBasePath();
} else {
    mainDirectory = argv[1];
}

loadFile("testfile.txt");

If I run the program with “/main/directory/” as the argument for example, the result of the final SDL_Log in the loadFile function prints out this:

stuff and things/main/directory//testfile.txt

Oddly enough, running the program with no arguments and using SDL_GetBasePath works perfectly fine. And rewriting the function so that it only uses stdio.h functions like fread and fclose also works fine.

Anyone know what’s going on here?

2

This expects fileContents to be a null-terminated string, so it’s a buffer overflow here. You should probably append a null-character to your buffer after you read the data.

Undefined behavior is a pretty random thing :grin:

1 Like
3

Can you try it with this code?

void loadFile(const char fileName[]) {
    char filePath[SDL_strlen(mainDirectory) + SDL_strlen(fileName) + 2];
    SDL_snprintf(filePath, sizeof(filePath), "%s/%s", mainDirectory, fileName);

    SDL_RWops* file = SDL_RWFromFile(filePath, "r");
    if (file == NULL) {
        SDL_Log("%s\n", "File could not be found in this directory.");
        return;
    }

    const Sint64 fileSize = SDL_RWsize(file);
    char fileContents[fileSize + 1]; // + 1 for null-char

    SDL_ClearError();
    size_t len = SDL_RWread(file, &fileContents, sizeof(char), fileSize);
    fileContents[len] = '\0'; // set null-char at end of the data
    if (len == 0) {
        const char* err = SDL_GetError();
        if (strcmp(err, "") == 0) {
            SDL_Log("%s\n", "Success, end of file.");
        } else {
            SDL_Log("%s\n", err);
        }
    }

    SDL_RWclose(file);
    SDL_Log("File contents: %s\n", fileContents);
}
1 Like
4

This works perfectly, thanks!

1 Like
5

By the way, while VLAs (char fileContents[fileSize + 1]) can be good for small allocations, it can cause stack overflow with larger fileSize.

1 Like