Pset4 recover: Images recovered but check50 failed

Question

I am currently stuck on this problem set, as it seems like I am able to recover the 50 jpgs but I am failing the check50. I am not sure what I am doing wrong and would appreciate some help. :( recovers 000.jpg correctly 000.jpg not found :( recovers middle images correctly 001.jpg not found :( recovers 049.jpg correctly 049.jpg not found

It also seems like the jpgs i have recovered are corrupted. This is my code for the problem:

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

int main(int argc, char *argv[])
{
    if (argc!=2 )
    {
        printf("Usage: ./recover image\n");
    return 1;
}

FILE *f= fopen(argv[1], "r");

if (!f)
{
    printf("File cannot be opened\n");
    return 1;
}

int count=0;
typedef uint8_t BYTE;

//make a buffer of size 512b
BYTE buff[512]={0};
//allocate memory for filenamed pic
char *pic=malloc(sizeof(char)*2048);
//intialise file pointer img 
FILE *img= NULL;

while(fread(buff, 512, 1, f))
//condition fulfilled to be a jpeg
{
    if(buff[0]==0xff && buff[1]==0xd8 && buff[2]==0xff && ((buff[3]&0xf0)==0xe0)  )
    {
            if((count=0))
            {
                sprintf(pic, "%03i.jpg", count);
                img=fopen(pic, "w"); 
                fwrite (buff, 512, 1, img);
                fclose(img);
                count++;
            }
            else if (count>0)
            {
                count++;
                sprintf(pic, "%03i.jpg", count);
                img=fopen(pic, "w"); 
                fwrite (buff, 512, 1, img);
                fclose(img);
            }       
    }


     //if not start of jpeg
    else
    {
        if(count>0)
        {

            fwrite(buff,sizeof(buff),1,img);
            fclose(img);

        }


    }



}

 free(pic);

}

Answer 1

There are a few issues going on here.

First, there's the test, as was previously stated.

        if((count=0))

This is an assignment, not a test. It'll set count to 0 every pass.

But the bigger problem is this. In the signature code blocks, the code opens the output file, writes the signature block, and then closes the output file! The output file needs to be left open so that additional non-signature blocks can be written to the output file!

Programming notes: As I said in a comment above, it would be more efficient, in the signature block code, to test whether a file is open and if open, close it. THEN, no matter what the value of count is, open a new output file. Any time you get two or more blocks of code that are identical, it's a red flag to check to see if the code can be written more efficiently, eliminating the duplicate blocks of code. It's not always possible, but should be checked.

Finally, there's this:

    char *pic=malloc(sizeof(char)*2048);

pic is a pointer to memory that will hold the name of the file to be created. The filename will be exactly 7 chars long, plus 1 char for the end of string marker, \0. That's 8 bytes. So, why allocate 2048 bytes for it? why not just allocate 8 bytes?

Take it a step further. Allocating dynamic memory during runtime is great for conditions where you have no idea how much memory will be needed overall, but when you know in advance exactly how much memory is needed, it's more appropriate and efficient to allocate regular stack variables. In this case, it would be less overhead and more efficient to just allocate a char array of 8 chars.

    char pic[8];

And remember to close all of the open files!

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

Answer 2

You have to work in your logic here.

First, the code in the if conditions if counter is 0 or not, is basically the same. It shouldn't be. If counter is 0, you have to create a new file, write the first 512 bytes and increment counter. If it's not 0, you have to close the previous file, create a new one and leave it open, in order to be able to write on it in the follow iterations.

Also you shouldn't close the file every time you write on it.

Don't forget to close both files when the while loop is finished.

Answer 3

Your count stays at zero. Change the following

 if((count=0))

to

 if((count==0))