End of File detection

Question

I’m a very novice programmer and I’ve encountered an issue whose nature I don’t understand, whilst working on a problem set of the excellent cs50 course. I have implemented a program to recover JPEG pictures from an image of a memory card and am implementing a break at End of File as follows:

if(file > 1)
    {
        if (fread(&buffer, 1, 512, in_pointer) != 512)
        {
            free(filename);
            return 0;
        }
        else 
            fseek(in_pointer, -512, SEEK_CUR); 
    }

(the pictures are filling up the card in 512 byte blocks). When I first implemented this it broke my first picture (it was recognizable but distorted) so I excluded it by means of the first if statement. Now however the middle files of the set are slightly off– they still open as Jpegs but I can’t get their thumbnails to work. My hypothesis is that I am corrupting the JPEG file format header. The beginning (including first and last images of the set work perfectly). My questions are:

1. What is an elegant way to implement an EOF break since my getto solution is causing trouble?

2. What is the likely nature of the problem I’ve created (in layman’s terms)?

Thank you very much,

Tikhon

ps here is the whole thing

#include <cs50.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdint.h>

int main(int argc, char *argv[])
{
// ensure proper usage
if (argc != 2)
{
    fprintf(stderr, "enter exactly two command line arguments: ./recover and destination of disc to scan\n");
    return 1;
}

//name the file
char *infile = argv[1];

//open card file and ensure proper format
FILE *in_pointer = fopen(infile, "r");
if (in_pointer == NULL)
{
    fprintf(stderr, "could not open %s\n", infile);
    return 2;
}

typedef uint8_t  BYTE;

BYTE buffer[512];
bool new_jpeg = false;
int block = 0;
int file = 0;
char *filename = malloc(3);
//sprintf(filename, "%03i.jpg",1);

do
{

    //read a 512 block of a jpeg
    fread(&buffer, 512, 1, in_pointer);

    //check for new jpeg
    if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0) // took me a while to figure this out
        {
    new_jpeg = true;
    //printf("jpeg found, block %i\n", block);
        }

    block++;



} while(new_jpeg == false);

do
{
            //set name of file to write to
            sprintf(filename, "%03i.jpg",file);
            file++;
            new_jpeg = false;

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


        //add blocks to file while before we reach the nea JPEG.
        do
        {


                fwrite(&buffer, 1, 512, img);

                //read the next block
                fread(&buffer, 1, 512, in_pointer);

                //There MUST be a better way... Anyhow this checks for end of file but backtracks becouse the act of checking moved the file coursor forward... 
                if(file > 1)
                {
                    if (fread(&buffer, 1, 512, in_pointer) != 512)
                    {
                        free(filename);
                        return 0;
                    }
                    else 
                    fseek(in_pointer, -512, SEEK_CUR); 
                }


                block++; //we are reading off teh next block

                if (buffer[0] == 0xff &&
                    buffer[1] == 0xd8 &&
                    buffer[2] == 0xff &&
                    (buffer[3] & 0xf0) == 0xe0) // took me a while to figure this out
                {
                    new_jpeg = true;
                    //printf("jpeg %i found, block %i\n", file, block);
                }


        }while(new_jpeg == false);

}while(!feof(in_pointer));    

free(filename);
//ran valgrind no probs detected.
}

Answer 1

OK, I fixed it.

Instead of having a whole separate section of code to check eof I killed two birds in one stone and fread the file WHILE checking for eof:

                //read the next block
                int k = fread(&buffer, 1, 512, in_pointer);
                if(k != 512)
                {
                        free(filename);
                        return 0;
                }

I still have no idea why my previous method didn't work, I would be extremly grateful for suggestions...