What is going wrong with fread() in my code?

Question

Here is an excerpt of the code I'm trying now in relation to my previous problems in cs50 'Recover'.

I've tried everything with fread, but for some reason it simply isn't working the way I want it to.

In this loop, I'm trying to figure out how fread actually works.

My question with fread is - does it read 512 bytes of data 1 at a time from file pointed to (rawdata) each time it's called? In that case, my code should work since the loop is running indefinitely, calling the function over and over and hence moving the stream position/file cursor (I don't know what its called) 512 bytes at a time. I have a break from the loop with feof(rawdata).

I'm using this small program to aid me with Recover from cs50 pset4.

    // In a loop, until the end of file has been reached,
    while (true) {
        // Zeroing counter
        jpg_counter = 0;
        // Reading 512 bytes into the array of bytes
        fread(bytes, 512, 1, rawdata);
        
        // Searching the 512 bytes for JPEG signatures - bytes[3] with bitwise AND
        if (bytes[0] == 0xff && bytes[1] == 0xd8 && bytes[2] == 0xff && (bytes[3] & 0xf0) == 0xe0) {
            jpg_counter++;
        }
        
        // If found JPG, add total and keep going till end of file
        if (jpg_counter != 0) {
            total++;
        }
 
        //feof returns a non zero value only at the end of a file
        if (feof(rawdata) != 0) {
            break;
        }
    }
    printf("%i\n", total);

Answer 1

There are 2 major problems with your approach:

• you do not correctly test for end of file in the reading loop. You should use the return value of fread to determine if you have read at least one byte from the stream, and you should pass the arguments in the opposite order: you are reading as many as 512 bytes:

    size_t n;
    while ((n = fread(bytes, 1, 512, rawdata)) > 0) {
        // n bytes were read from the file.
  

• you only test the first 4 bytes for a JPEG signature. Unless the file is known to have a very specific structure, you probably want to search the block for signatures.

• care must be taken to handle signatures overlapping blocks, ie: that start at the end of a 512 byte block but end on the next block.

Here is a modified version:

int count_jpeg(FILE *rawdata) {
    unsigned char bytes[3 + 512];
    int pos = 0, end, i;
    int total = 0;
    size_t nread;
    // In a loop, until the end of file has been reached,
    while ((nread = fread(bytes + pos, 1, 512, rawdata)) > 0) {
        end = pos + nread - 3;
        for (i = 0; i < end; i++) {
            // Searching the block for JPEG signatures - bytes[3] with bitwise AND
            if (bytes[i] == 0xff && bytes[i + 1] == 0xd8
            &&  bytes[i + 2] == 0xff && (bytes[i + 3] & 0xf0) == 0xe0) {
                total++;
                i += 3;
            }
        }
        // copy the last 3 bytes to the beginning of the block
        for (i = pos = 0; i < 3; i++)
            if (end + i >= 0) {
                bytes[pos++] = bytes[end + i];
        }
    }
    printf("%i\n", total);
    return total;
}

EDIT: if the JPEG signatures are known to be aligned on 512-byte boundaries, the scanning loop can be removed, but reading a partial block at the end of the file should still be possible:

int count_jpeg(FILE *rawdata) {
    unsigned char bytes[512];
    int total = 0;

    // In a loop, until the end of file has been reached,
    while (fread(bytes + pos, 1, 512, rawdata) > 4) {
        /* check for a signature at the beginning of the block */
        if (bytes[0] == 0xff && bytes[1] == 0xd8
        &&  bytes[2] == 0xff && (bytes[3] & 0xf0) == 0xe0) {
            total++;
        }
    }
    printf("%i\n", total);
    return total;
}

Answer 2

man page of fread states

The function fread() reads nmemb items of data, each size bytes long, from the stream pointed to by stream, storing them at the location given by ptr

so you can store number of elements read in a variable and check if 512 bytes are read.

int nmenb = fread(bytes, 512, 1, rawdata);

if( nmemb != 1 )
{
  //exit
}
else{
      // 512 bytes read successfully 
}