CODEWITHSUNDEEP
cfopen
Winfield Trail
Winfield Trail

Reputation: 5695

Gibberish after file contents

I'm working on the following function to pull scripts into my C application:

char *anne_script_load(char fileName[]){
    char path[6400] = "data/scripts/";
    strncat(path, fileName, 6000);
    strncat(path, ".lua", 5);
    struct stat fileInfo;
    char *contents = NULL;
    FILE * pFile;

    stat(path, &fileInfo);
    contents = (char*)malloc(fileInfo.st_size);
    pFile = fopen (path,"r");
    fread (contents,1,fileInfo.st_size,pFile);
    fclose(pFile);

    printf("Script path: %s\n", path);
    printf("Script loaded: %s\n", contents);
    return contents;
}

At runtime, the second printf generates the following output:

test script - if you see this load is working :) helicopters����

The garbled text looks different on my console, but I'm not sure it matters: my theory is that the file stream doesn't end in a null byte (it's not stored on disk as a c string, after all - so I terminated it myself as follows:

contents[fileInfo.st_size] = 0;

This appears to work, but I'm concerned about the robustness of this solution. Is there a better, generally accepted way of doing this?

Upvotes: 0

Views: 948

Answers (3)

yeyo
yeyo

Reputation: 3009

You must nul-terminated the string otherwise printf() will not know for sure when to stop reading from the pointer you just passed. The same applies to other functions like strcmp(), you could have two strings with the same content while having strcmp() yelling non-zero because of a non nul-terminated string.

So contents[fileInfo.st_size] = 0; is just fine.

You could read a little bit more about nul-terminated strings on Wikipedia

Upvotes: 1

Charlie Burns
Charlie Burns

Reputation: 7054

You need to add +1 to your malloc for the termination:

if(stat(path, &fileInfo) != 0) {
   perror("stat");
   ...
}
contents = (char*)malloc(fileInfo.st_size + 1); // don't forget this
pFile = fopen (path,"r"); 
if(pFile == 0) {
    perror("fopen");
    ...
}
int n = fread (contents,1,fileInfo.st_size,pFile);
if(n != fileInfo.st_size) {
   perror("read");
   ...
}
contents[n] = 0; // terminate after what you read. Not what you think you read.

** Check the return values from fopen, stat, and read.**

Upvotes: 3

Peter Cardona
Peter Cardona

Reputation: 2110

There are number of things you could probably improve here... but to answer your question, fread() is reading bytes into an array that you didn't initialize. In C, you can't expect a \0 character after the last byte you read via fread -- you've got to put it there first, either with a memset() or calloc().

Also, if the file content is treated as a text string, be sure to allocate one additional byte over the size to hold that terminating \0 character!

Upvotes: 1

Related Questions