Read binary file, save in buffer, print out content of buffer

Question

I have a big problem that need's to be solved before I can continue with my program.

I have to open a binary file, read it's content, save the content into a buffer, allocate space on the heap with malloc, close the file and finally printf( the content of the .bin file). I came this far (closing file is not implemented yet):

void executeFile(char *path){
    FILE *fp; /*filepointer*/
    size_t size; /*filesize*/
    unsigned int buffer []; /*buffer*/

    fp = fopen(path,"rb"); /*open file*/
    fseek(fp, 0, SEEK_END); 
    size = ftell(fp);         /*calc the size needed*/
    fseek(fp, 0, SEEK_SET); 
    buffer = malloc(size);  /*allocalte space on heap*/

    if (fp == NULL){ /*ERROR detection if file == empty*/
        printf("Error: There was an Error reading the file %s \n", path);           
        exit(1);
    }
    else if (fread(&buffer, sizeof(unsigned int), size, fp) != size){ /* if count of read bytes != calculated size of .bin file -> ERROR*/
        printf("Error: There was an Error reading the file %s - %d\n", path, r);
        exit(1);
    }else{int i;
        for(i=0; i<size;i++){       
            printf("%x", buffer[i]);
        }
    }
}

I think I messed up the buffer and I am not really sure if I read the .bin file correctly because I can't print it with printf("%x", buffer[i])

Hope you guys can help

Greetings from germany :)

Detraction

Where is size being set? You have it commented out. – Multinuclear

Just for that case when things don't work out perfectly (like, right now for example), you may wish to consider checking the return value of the functions you call. fopen(), malloc(), fseek().. None are being checked, though you do eventually check for fp == NULL, but only after you blindly fseek() on it, and the NULL check appears to be a file-empty check (which isn't correct either) rather than failure-to-open. Read the library documentation. – Mentalist

Commented size out per accident->fixed – Detraction

I started writing up a detailed answer, but just do this: (1) Check return values, (2) Change buffer base type to unsigned char *buffer;. (3) Fix fread() call to pass buffer as the first parameter (not &buffer), and sizeof(buffer[0]) as the second. and finally (4) close your file with fclose() and free your buffer with free(). Optional checking for size == 0 before entering into the read-and-print I leave to you. – Mentalist

1. checked return values as far as i could ->everything is fine ?! 2. calculation of the filesize was given by my prof so this workes 3. fopen() returns FILE if correct and NULL if not, so checking if fp == NULL is a way of error-checking while opening the file 4. I did not inserted size == 0 check before priting out because it is just to test and see what's in the buffer. will be erased if everything is ok 5. i will use free() later on. I need to work with this buffer. – Detraction

Also note that fstat() might be a better way to determine file size than multiple fseek() calls... – Joviality

I did everything that WhozCraig said I should do. I have a problem with my buffer. It need's a size when it's beeing initialized but I can't give it a size because idk how big the file is yet. it is calculated 4 lines under the declaration of my buffer. so what should I do ? – Detraction

@Joviality I concur fstat() is a better than the pair of fseek() and ftell(). Either gets the job done, so my below answer focused on needed changes. – Heaney

If you are using C99 (or C11), you could use a VLA (variable length array) by determining the size and then defining unsigned int buffer[size / sizeof(int)]; in the middle of the function. That brings up another point; you are reading into an array of int from a file sized in terms of bytes (char, roughly); hence the compensating / sizeof(int) in my suggestion. – Affairs

unsigned int buffer[size / sizeof(int)]Using a buffer of type int, where sizeof(int) could be greater than 1 imposes an additional concerns. Since the file may not have a .> 1 – Heaney

Please ignore previous comment. Using C99 or C11 and buffer of type int, where sizeof(int) could be greater than 1 imposes an additional concern. Since the file byte size may not be a multiple of sizeof(int), one would need to define a slightly larger "unsigned int buffer[(size + sizeof(int) -1)/ sizeof(int)];" to cope with pesky odd additional bytes. But a buffer of type int could work - just some issues. – Heaney

should read files in chunk? what if the file is too large? – Vineland

Answer 1

Recommended changes:

1. Change your buffer to a char (byte), as the ftell() will report the size in bytes (char) and malloc() use the byte size also.

   unsigned int buffer []; /buffer/

to

unsigned char *buffer; /*buffer*/

2. [Edit] 2021: Omit cast
   2) This is OK, size is bytes and buffer points to bytes, but could be explicitly cast

   buffer = malloc(size); /allocate space on heap/

to

buffer = (unsigned char *) malloc(size);  /*allocate space on heap*/
/* or for those who recommend no casting on malloc() */
buffer = malloc(size);  /*allocate space on heap*/

3. change 2nd parameter from sizeof(unsigned int) to sizeof *buffer, which is 1.

   else if (fread(buffer, sizeof(unsigned int), size, fp) != size){

to

else if (fread(buffer, sizeof *buffer, size, fp) != size){ 

4. Change "%x" to "%02x" else single digit hexadecimal numbers will confuse the output. E. g. is "1234" four bytes or two?

   printf("%x", buffer[i]);

to

printf("%02x", buffer[i]);

5. Your clean-up at the end of the function may include

   fclose(fp); free(buffer);

Heaney

You can't use an empty array bound like that inside a function (OK in an argument list). And you can't assign to an array like the malloc() does. It needs to be unsigned int *buffer;. – Affairs

yes he is complaining about the empty array bound ... the program does not know how big the buffer has to be because it doesn't know what size the file has ... – Detraction

@Jonathan Leffler : corrected solution from "unsigned int buffer[]" to "unsigned char *buffer". – Heaney

@luizfls Why yes it is! Code amended. – Heaney