really sorry, not sure how to format copy-and-pasted code so it renders correctly in this forum. I'm hoping for a hint or two on how to get my Recover program (PSet 3, 2018) to work. I'm able to get it to compile and open 50 .jpg files (appropriately named), however they seem "empty" (gray and white pixelated images of varying sizes), and upon running check50 I get the following results. I'm not really sure what I'm doing wrong.

One other question I have is why my boolean expression didn't originally work. I've kept the code there for reference, but ended up just putting the jpg header info in an "if" statement.

Appreciative of any hints/guidance, and hopeful that my stumbles can help others in similar positions!

:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:( recovers 000.jpg correctly
    000.jpg not found
:( recovers middle images correctly
    recovered image does not match
:( recovers 049.jpg correctly
    recovered image does not match

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h> // Note: this was stored in separate bmp.h file for Resize, Less
#include <cs50.h> // so bool works
#include <stdbool.h>

// data type for buffer, of size "BYTE" (8 bits)
typedef uint8_t BYTE; // from stdint.h

int main (int argc, char *argv[])
    if (argc != 2) // checks arg count
       fprintf(stderr, "Usage: ./recover infile\n");
       return 1;

    if (argv[1] == NULL)
        fprintf(stderr, "Null...\n");
        return 2;

    // remember filename, open input file (card.raw) Note: "r" stands for read
    char *inputfile = argv[1];
    FILE *card = fopen(inputfile, "r");
    if (card == NULL)
        fprintf(stderr, "Could not open %s.\n", inputfile);
        return 3;

    // define buffer array of size 512 bytes, as is "block size"
    BYTE buffer[512];
    FILE *image = NULL;

    // initialize variable to count images, to be incremented upon the discovery of each new image (1 - 50)
    int imageCount = 0;

    // define boolean for jpg header file
    //bool imageHeaderFound = (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0);

    while (fread(&buffer, sizeof(buffer), 1, card) == 1)
        // while jpg header file found, do the following....
        //if (imageHeaderFound == true)
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
            if (imageCount > 0)

            // increment image count so filename can be updated
            imageCount ++;

            // name jpg file  - "023.jpg" if imageCount is at 23
            char outputfile[8];
            sprintf(outputfile, "%03i.jpg", imageCount);

            // open jpg file
            image = fopen(outputfile, "w");
            if (image == NULL)
                fprintf(stderr, "Could not open %s. \n", outputfile);
                return 3;

            // start writing to the new jpg file in chunks of 512 B (until start of new jpg found...)
            fwrite(&buffer, sizeof(buffer), 1, image);
    return 0;

You're getting there.

In glancing through your code, here's what I see as the major problem. The code is finding data blocks with a signature in the first 4 bytes, opening a new file and writing the block out to the file. The problem is that it doesn't do anything with the data blocks that follow that don't contain signatures at the beginning. All that valid image data is being ignored. When you open one of the files that you created, the header data in the first 512 bytes is telling the system to read x bytes. The system pulls that data from the file location on the drive. But since the valid data isn't there, it's actually pulling the garbage data in the slack space at the end of the file that fills out the sector on the disk.

Image files are never just 512 bytes long! As the program spec said, all the data that follows the first signature is valid image data. That means that once a signature is found, all the data blocks that follow are valid image data blocks and must be written to the open jpg file. Only when a new signature is found (which your code does) is a file closed and a new file opened.

In short, you need to add code to handle the intermediate data that follows the signatures.

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

  • Thank you! That helped me go back and think about when and what I'm actually asking the program to write to new image files. (I added a "jpgFound variable, initialized it to 0 initially, changed it to 1 when jpgHeader found...) I'm now able to recover 50 images (real images! with real people in them!), but when I run Check50, the final three checks return red frowny faces, indicating that the found images don't match. Wondered for a second if I was recover 2019 images from 2018 problem set, but tried check50 with both years and same results. Any ideas about what might be happening here? – LippStick Oct 11 '19 at 16:26
  • Note: Exactly 50 images were recovered, and they are named 001.jpg through 050.jpg. First image recovered is of a red-headed person sitting on a log. – LippStick Oct 11 '19 at 16:34
  • Well, that would be a problem and would cause 100% fail. The images should be numbered from 000 to 049. Since you're starting at 001, no image will match the correct answer image. The filenames are mismatched, offset by 1 from the correct numbered image. – Cliff B Oct 11 '19 at 16:37
  • WAIT, just answered my own question. (Thanks for the quick response, in the meantime!) Yup: problem was with naming. I need to be more careful with reading directions. Changed the position of my "image count incrementer" to come later and now all working fine. I should've seen the glaring hint in check50 that "image 000.jpg" doesn't match... meaning, there should've been an image named "000.jpg" which I didn't originally have. – LippStick Oct 11 '19 at 16:43
  • I will say, it's a little funny that that error (which seems pretty minor?) would result in a check score of only 50%. The upside is that it's an easy fix to get to 100%, but seems like the check command in this problem set could be designed to be a bit more nuanced. Just my two cents, in the event any CS50 staff are reading this! – LippStick Oct 11 '19 at 17:40

You must log in to answer this question.

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