0

I think my "while" condition to read till the end of the file seems correct unlike most of other answers so i'm not sure what is wrong

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

int main(int argc, char *argv[])
{
    //this counter to keep track of how many jpegs found so far
    int counter = 0;
 
    //allocate a place with 8 characters for inputting the filename 
    char filename[8];
    //replace argv[1] with card
    char *card = argv[1];        
    //create a pointer called img 
    FILE *img;
    
    

    //check that if user did not give any inputs, to return
    if(argc !=2)
    {
        printf("Usage: ./recover image \n");
        return 1;
    }

    //open up file and read the file
    FILE *file = fopen(card,"r");
    if(!file)
    {
        printf("Could not open card\n");
        return 1;
    }

    //declare a buffer to put the content that i'm current reading 
    unsigned char buffer[512];

    //iterate over contents of the file till the end of the file, since ask to read the element once, if it does not read 512, then it will read it once
    while (fread(buffer, sizeof(buffer), 1, file) == 1)
    {
        //if first 4 bytes of the array are as per the below
        if(buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            //if this is the 1st JPEG that someone is working on
            if(counter == 0)
            {
                //create a new jpeg file
                sprintf(filename, "%03i.jpg", counter);
                counter++;


                //open this new jpeg file
                img = fopen(filename, "w");

                //start writing to this new jpeg
                fwrite(buffer, sizeof(buffer), 1, img);
            }
            
            //if this is not the 1st JPEG
            else if (counter > 0)
            {
                //close the file at location img that was being written to 
                fclose(img);
                
                //create a new jpeg file
                sprintf(filename, "%03i.jpg", counter);
                counter++;

                //open this new jpeg file
                img = fopen(filename, "w");

                //start writing to this new jpeg, need a number that is based on how many times before the next "start" of the jpeg file
                fwrite(buffer, sizeof(buffer), 1, img);
                
            }
            
            
        }
        
        //to continue writing if there was a JPEG found before previously 
        else if (counter > 0 && buffer[0] != 0xff )
        {
            //continue writing to this new jpeg
            fwrite(buffer, sizeof(buffer), 1, img);
        }
        
        
        
    }

    //close files
    fclose(img);
    fclose(file);


}
1

Look at this line of code:

    else if (counter > 0 && buffer[0] != 0xff )

Why couldn't a single byte of data in an image be 0xff?

Clearly, you were trying to put in a check to make sure it isn't a signature block, but it's simply impossible for a signature block to get to this code. The extra test isn't needed.

This test will (and does) only serve to possibly remove blocks from the code and to corrupt images.

If this answers your question, please click on the check mark to accept. Let's keep up with forum maintenance. ;-)

| improve this answer | |
  • Thanks Cliff! I was thinking that i had a condition for (if the 512 block) was the start of the jpeg and needed another condition for if it is not a start of the jpeg. When i just use "Else", i got alot more errors and a segmentation fault. Trying to think what condition to put. Are you saying that a condition isn't necessary? – Daryl Jun 25 at 3:44
  • No. There is one condition (not two) that's absolutely necessary in order to write to a file. – Cliff B Jun 25 at 3:48
  • Thanks CLiff, that worked for me! – Daryl Jun 25 at 4:06

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .