CS50 Recover - fread into buffer

Question

I am working on CS50 Recovery and I am unable to get the program to check the JPEG starting header.

I used debug50 and added a couple print statements to try and find the problem. Every time I run the program, it prints out "No Output File 2" meaning no outfile is created, and the IsJPEG boolean check is skipped.

Upon further debugging it seems that the buffer is 0'\000\ all along, so I suspect nothing is written into the buffer, causing the program to skip the boolean check (which trigger fopen) and hence causing it to break.

However, I struggle to find the reason why nothing is written into buffer.

Please help and thank you!

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

typedef uint8_t BYTE;
bool isJPEG(BYTE buffer[]);
int main(int argc, char *argv[])
{
    BYTE buffer[512];
    int fileCounter = 0;
    char filename[8];
    if (argc != 2)
    {
        printf("%s\n", "Usage: ./recovery file");
        return 1;
    }
    FILE* infile = fopen(argv[1], "r");
    if (infile == NULL)
    {
        printf("%s\n", "Error: File Not Found");
        return 1;
    }
    FILE* outfile;
    while (fread(buffer, 512, 1, infile))
    {
        if (isJPEG(buffer))
        {
            if (fileCounter == 0)
            {
                sprintf(filename, "%03i.jpg", fileCounter++);
                outfile = fopen(filename, "w");
                printf("%s\n", "Bool 1");
            }
            else
            {
                fclose(outfile);
                sprintf(filename, "%03i.jpg", fileCounter++);
                outfile = fopen(filename, "w");
                printf("%s\n", "Bool 2");
            }
            if (outfile == NULL)
            {
                printf("%s\n", "No Output File 1");
                return 1;
            }
            fwrite(buffer, 512, 1, outfile);
        }
        else
        {
            if (outfile == NULL)
            {
                printf("%s\n", "No Output File 2");
                return 1;
            }
            fwrite(buffer, 512, 1, outfile);
        }
    }
    fclose(outfile);
    fclose(infile);
}

bool isJPEG(BYTE buffer[])
{
    return buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0;
}

Now I have moved this code block downwards and it works, but I have no idea why

BYTE buffer[512];
int fileCounter = 0;
char filename[8];

This is the revised code:

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

typedef uint8_t BYTE;
bool isJPEG(BYTE buffer[]);
int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("%s\n", "Usage: ./recovery file");
        return 1;
    }
    FILE* infile = fopen(argv[1], "r");
    if (infile == NULL)
    {
        printf("%s\n", "Error: File Not Found");
        return 1;
    }
    BYTE buffer[512];
    int fileCounter = 0;
    char filename[8];
    FILE* outfile;

Answer 1

I copy-pasted your code on CS50 IDE and used the debugger. The first 512-bytes block is not a JPEG so I go to line 51 (after the else) and test outfile==NULL. You can see that the pointer to the outfile is NULL so it makes sense that your program is entering the if statement and printing "No Output File 2".

You are using the correct functions but you have a problem with the logic/ordering. Remember the following from the problem statement:

• Open Memory Card (which you are already doing)
• Read 512-bytes at a time (which you are already doing)
• If JPEG found, open a new JPEG. If this is your second or more JPEG you need to close the previous JPEG.
• Keep writing until you find another JPEG

Answer 2

Look at the following code:

    else
    {
        if (outfile == NULL)
        {
            printf("%s\n", "No Output File 2");
            return 1;
        }
        fwrite(buffer, 512, 1, outfile);
    }

Now, remember what the pset spec said. The data at the beginning of the input file is garbage and needs to be discarded. That guarantees that the first read will not have a signature. Also, only the first block of data in any given file will have a signature. After the first signature is found, no more garbage and subsequent blocks need to be added to the open output file until another signature block is found.

Looking back at the code, it does a fine job of identifying when there is or isn't a signature at the start of a 512 byte block. The problem is that there is a return statement in the else statement. It will terminate the program immediately, instead of making a decision on what to do with the block.

Try recoding to handle non-signature blocks correctly. It's actually a couple of minor changes and you're done! ;-)

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