fread returns same number as nmemb yet end of file is reached

Question

I am completing a problem in CS50, and my code is successful although I don't understand the behavior of a test inside of it. Line 63 if (feof(inptr))checks if the end of file is reached, and then I ask to print the size of a buffer pointer which should be less than what it was initialized to (512). It still returns a value of 512 although the EOF is reached, which doesn't make sense.

Could someone tell me what is wrong?

#include <stdio.h>
#include <string.h>


int main(int argc, char *argv[])
{
    // ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: copy infile outfile\n");
        return 1;
    }

    // remember filenames
    char *infile = argv[1];
    char *outfile = "000.jpg";

    // open input file
    FILE *inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", infile);
        return 2;
    }

    // open output file
    FILE *outptr = fopen(outfile, "w");
    if (outptr == NULL)
    {
        fclose(inptr);
        fprintf(stderr, "Could not create %s.\n", outfile);
        return 3;
    }

    // declaring variable
    unsigned char buffer[512];
    int count = 0;
    int test = 512;

    // Execute until we find end of card
    while (!feof(inptr))
    {

        // Read buffer in card
        fread(buffer, 1, sizeof(buffer), inptr);

        // Checks for jpeg signature
        if (buffer[0] == 0xff &&
            buffer[1] == 0xd8 &&
            buffer[2] == 0xff &&
            (buffer[3] & 0xf0) == 0xe0)
        {
            fwrite(buffer, 1, sizeof(buffer), outptr);
            fread(buffer, 1, sizeof(buffer), inptr);

            // Checks if we are still in a jpeg, not the beginning of new one
            while (buffer[0] != 0xff ||
            buffer[1] != 0xd8 ||
            buffer[2] != 0xff ||
            (buffer[3] & 0xf0) != 0xe0)
            {
                // Exits loop if end of file
                if (feof(inptr))
                {
                    int size = sizeof(buffer);
                    printf("%i\n", size);
                    break;
                }

                fwrite(buffer, 1, sizeof(buffer), outptr);
                fread(buffer, 1, sizeof(buffer), inptr);

            }
            if (feof(inptr))
            {
                break;
            }

            // Close jpeg
            fclose(outptr);

            // Change count to apply to next jpeg title
            count++;

            char img_num[4];
            sprintf(img_num, "%03i.jpg", count);

            // Assign new title to new jpeg
            outfile = img_num;
            printf("%s\n", outfile);
            outptr = fopen(outfile, "w");

            // We will have to read again in the main loop, so rewind
            fseek(inptr, -512, SEEK_CUR);
        }


    }

    printf("%i\n", test);

    // close infile
    fclose(inptr);

    // close outfile
    fclose(outptr);

    // success
    return 0;
}

Best to read .jpg files in binary mode `fopen(infile, "r");` --> `fopen(infile, "rb");`. — chux - Reinstate Monica, Jul 31 '18 at 23:50
Drop the `feof()` and check the return value from `fread()` instead. — chux - Reinstate Monica, Jul 31 '18 at 23:51
[while(!feof(fp)) is always wrong](https://stackoverflow.com/questions/5431941) — user3386109, Jul 31 '18 at 23:54
Binary files must be opened in binary mode ("rb"). I suppose you're using Windows, so it really matters. Reading or writing binary data in text mode will result in data corruption. — rici, Aug 01 '18 at 05:54

score 3 · Accepted Answer · answered Jul 31 '18 at 23:54

3

sizeof(buffer) tells you how big buffer is. It does not tell you anything about what was in it—not how many bytes are currently valid, and not how many were read in the last fread.

The proper way to know how many bytes fread read is to use its return value. You ought to use code such as:

size_t BytesRead = fread(buffer, 1, sizeof(buffer), inptr);

If, after this statement, BytesRead is less than sizeof buffer, then fread did not read all the bytes you asked for. That is an indication that something is wrong, and it is a better indication than whether the file EOF flag is set.

answered Jul 31 '18 at 23:54

Eric Postpischil

195,579
13
168
312

Thanks for the input. I actually tried comparing the return value of fread with the value of sizeof(buffer), but they always were the same. I assumed they weren't different but you helped me understand how different they are. I still end up with corrupt files (jpeg outputs) when implementing this solution on if statements instead of looking for the EOF flag. The return value goes to 0, sometimes before the EOF flag. Anyway, time for something else. – PYB Aug 01 '18 at 00:28

score 0 · Answer 2 · answered Aug 01 '18 at 00:00

(sizeof) returns size in bytes of the object representation of type

https://en.cppreference.com/w/cpp/language/sizeof

Since you allocated 512 bytes for buffer, sizeof will return 512. It doesn't matter what buffer is holding since its not a length check.

fread returns same number as nmemb yet end of file is reached

2 Answers2