CS50 Recover (pset4). Images dont seem to load

Question

my program compiles successfully but it doesn't seem to produce any images at all. I tried using debug50 and it seems like the if condition for buffer[0] through to buffer[3] isn't fulfilled, or the buffer array isn't containing the card.raw data. I am unsure about the exact problem or how to solve it, some help would be much appreciated :)

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    if (argc == 2)
    {
        if (strcmp(argv[1], "card.raw") != 0)
        {
        printf("Usage: ./recover image\n");
        return 1;
        }
        
        char* get_file = argv[1];
        FILE *file = fopen(get_file, "r");
      
        char filename[7];
        int i = 0;
        
        unsigned char buffer[512] = {0};
        
        char ch;
        while ((ch = fgetc(file)) != EOF)
        {

            fread(buffer, sizeof(BYTE), 1, file);
            
            if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
            {
                if (i == 0)
                {
                    sprintf(filename, "%03i.jpg", i);
                    FILE *img = fopen(filename, "w");
                    fwrite(buffer, sizeof(BYTE), 1, img);
                    fclose(img);
                    i++;
                }
                else
                {
                    sprintf(filename, "%03i.jpg", i);
                    FILE *img = fopen(filename, "w");
                    fwrite(buffer, sizeof(BYTE), 1, img);
                    fclose(img);
                    i++;
                }
            }  
            else if (i != 0)
            {
                FILE *img = fopen(filename, "a");
                fwrite(buffer, sizeof(BYTE), 1, img);
                fclose(img);
            }
        }
    }
    
    else
    {
        return 1;
    }
    return 0;
}

Answer 1

Think about what this code does:

    while ((ch = fgetc(file)) != EOF)
    {

        fread(buffer, sizeof(BYTE), 1, file);

First, fgetc reads a single char from the input file. Then the fread reads 1 byte. That first read is moving the starting point for the read into buffer. This means that no signature will ever be found. But since the fread is only reading 1 byte and not 512 bytes, the fread is broken anyways.

Think about how to read 512 bytes at once, and how to incorporate the fread into the while statement. Start by reading the man page for fread and/or google fread. Pay close attention to the return values.

Also, there's this:

    char filename[7];

The filenames that are being created have 7 chars. But strings are always stored with an end of string marker, '\0'. There needs to be room for that, so it should be 8, not 7.

And why does the code do a check to see if the input file name is card.raw? You might as well hardcode it. What if the input file name needed to be different? Was there something in the program spec that said to check the input file name????

Now, why does the code close the output file shortly after opening it? The output file should only be closed when a new signature is found, so that the next file can be opened.

THere may be more issues, but this will keep you busy for a while. ;-)

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