pset 4 recover : all images are grey and white boxes

Question

My recover program compiles and runs (finds 27 images). While opening them they all seem to be different at least in shape. However the copying seems to be problematic (all made of white and grey squares instead of proper block)

I have been on this forever, I feel like it's the last step :)

thanks for the help !

    // check argv1 exists

    char *infile = argv[1];

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


    uint8_t buffer[512];
    char filename[8];


    while (fread(&buffer, sizeof(buffer), 1, inptr) == 1) // read as long as we haven't reached EOF
    {

        fread(&buffer,512, 1, inptr); // read input file

        // condition if we meet a start of JPEG signature
        if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        (buffer[3] & 0xf0) == 0xe0)
        {
            if(count!=0)
            {
                count = count+1; // increase count of images found
                FILE *img = fopen(filename,"w");
                fclose(img); // close file we were working on
            }

            if (count==0) // manage case for first image found
            {
                printf("%i loop 1 \n",loop); // for testing purposes

                sprintf(filename, "%03i.jpg",count);
                FILE *img = fopen(filename,"w");
                fwrite(&buffer,512,1,img);
                count = count+1; // increase count of images found

            }
            if (count > 1)
            {

                printf("%i loop 2 \n",loop); // testing purposes
                printf("we found %i images\n", count); // testing purposes
                sprintf(filename,"%03i.jpg",count);
                FILE *img = fopen(filename,"w");
                fwrite(&buffer,512,1,img);
                // fclose(img);

            }
        }
        else if(count!=0) // keep printing in the same file if we haven't met a new signature

        {
            FILE *img = fopen(filename,"w");
            fwrite(&buffer,512,1,img);

        }


    }
    printf("end of while loopfile reached\n"); // for debug purposes

// close file
// fclose(img):
fclose(inptr);

Answer 1

There are a lot of issues with this code. First, there are two fread statements at the top of the while loop, one in the while statement and one immediately following. This discards half the data. When an fread call (or any other function call) is embedded inside a while statement, it isn't just a test. It also executes a read.

Next, it appears that there is an fopen statement paired to any fread statement. A file needs to be opened only once to write to it in multiple places. It will remain open until closed.

In a related issue, look at the following:

        if(count!=0)
        {
            count = count+1; // increase count of images found
            FILE *img = fopen(filename,"w");
            fclose(img); // close file we were working on
        }

This reassigns the img pointer to a new file being opened for write. If its the same file, because it's opened for write, it will erase the current contents. If it is a different file, the original file access is lost and a new file is opened, similar to a memory leak. Of course, this code just closed the newly opened file with no content.

Next are the if(count==0) and if(count > 1) code blocks. They essentially do exactly the same thing, except for incrementing count when it probably should (depending on other code corrections). And what if count == 1?

If your code is opening 27 files, the number is way off.

There's a lot of redesign needed here, and I expect there are other issues not covered here, but you need a chance to work on this and work through them. Actually, this is a good thing. You'll get practice refining and debugging code logic, a powerful tool to have later!

After your redesign, please feel free to open a new question to address any new problem.

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