Recover: Only finding 2 images both are correct

Question

Have spend the last 5 hours trying to figure out what am i doing wrong with my code, I seem to fail to understand what the mistake is in my logic, I am getting just 2 images as 000.jpg and 001.jpg both are correct, check50 is passing the first three tests but failing the last 2 tests obviously since there are only 2 files created but can't seem to understand why...HELP PLZZZZ

   /**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 *
 * Recovers JPEGs from a forensic image.
 */

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

typedef uint8_t ONEBYTE; 

int main(void)
{
    // opening card
    FILE* infile = fopen("card.raw", "r");

    if(infile == NULL)
    {
        printf("could not open card.\n");
        return 1;
    }

    /*declaring an array of characters to store 512 characters/bytes since char* points to 
     1st byte of array and the next bytes would follow consecutively */
    ONEBYTE buffer[512];

    //to track number of jpeg
    int counter = 0;

    // a filename where the string would be stored as filename
    char filename[7];

    //int c;
    while(!feof(infile))
    {
        //reading data from card pointed by pointer infile in chunks of 512 Bytes one at a time and storing to buffer
        fread(&buffer, 512 , 1, infile);


        if((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe0) || (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe1))
        {
            sprintf(filename, "%03i.jpg", counter);
            FILE* outfile = fopen(filename, "w");
            if(outfile == NULL)
            {
                printf("could not create a jpeg file.\n");
                return 2;
            }    

            counter++;
            // writing to outputfile
            fwrite(&buffer, 512, 1, outfile);

            int i= 0;
            while(counter !=0 && i== 0)
            {
                //to start reading after the first header of jpeg is found
                fread(&buffer, 512, 1, infile);

                //if it's the end of file than closing the last opened file
                if(feof(infile) == EOF )
                {
                    fclose(outfile);
                    i = 1;
                }
                //if its the start of a new jpeg
                else if((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe0) || (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe1))
                {
                    //closing the previous output file
                    fclose(outfile);
                    i = 1;
                }
                else 
                {
                    fwrite(&buffer, 512, 1, outfile);
                }
            }

        }





    }

    fclose(infile);
    return 0;
}

Answer 1

The question may be regarded as too broad, since errors are many, devise an algorithm before writing a program is one of the things that is intended in this course, is the most difficult (and most interesting) of the programming, no matter what language you use. First Error: filename has at least eight characters, note that is a string and must take into account that conclude with the character \0, your only reserves 7 bytes, insufficient. What our friend @Jon points, it is not a mistake but can do so much simpler. Second Error:

 while(!feof(infile))

It is not the right way to end a loop, there is much discussion about this here and in reddit, the rest of the program get tired, you read the file twice and check twice the beginnings of jpg files, writing under certain conditions IF-ELSE, I have not checked thoroughly, but I'm pretty sure they do not work like you think. My advice is to think of an algorithm to solve the problem and try to carry it out, I'll give an algorithm is pseudocode if you want to try

• initiate image counter to zero
• initiate a flag to zero

• while fread read something (a question about how to do this is a good question)

  • IF the jpg file is open we closed it
    • sprint title
    • open new jpg
    • increase counter
    • we make flag 1

  // end IF

  • IF flag is 1

  • we write in jpg

// end while

• fclose files

//end main

Answer 2

It might be because in this line if((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe0) || (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && buffer[3] == 0xe1)) you are only checking for 0xe0 and 0xe1 for the fourth byte. On the pset, it says the fourth byte can be 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5 all the way to 0xef. I hope this helps!