CS50 recover - images are some kind of blank

Question

//My program creates images of different sizes but without content (just black and white pixels). Someone can help me?

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

typedef uint8_t BYTE;

bool JPEGstart(BYTE buffer[]);

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

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


    BYTE *buffer = malloc(512);
    int COUNT = 0;

    do
    {
        if (JPEGstart(buffer) == true)
        {
            COUNT++;
            char *filename = malloc((7+1));
            sprintf(filename, "%03i.jpg", COUNT);
            FILE *img = fopen(filename, "w");

            do
            {
                fwrite(buffer, 512, 1, img);
            }
            while (JPEGstart(buffer) == false);

            // close outfile
            fclose(img);
        }
    }
    // read raw_file and checking if the end of card.raw was reached
    while (fread(buffer, 512, 1, raw_file) == 1);
}


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

        else
        {
            return false;
        }
}

Answer 1

The code does not match your description of "images of different sizes". Maybe sizes in pixels, but the file size generated by this code will always be exactly 512 bytes.

Some odd things.

    BYTE *buffer = malloc(512);
    int COUNT = 0;

    do
    {
        if (JPEGstart(buffer) == true)

You allocate a buffer. Then you test the buffer content before you read any sector. Better use a while instead of do..while.

And ALL_CAPS in C by convention is used for constants, not variables.

            do
            {
                fwrite(buffer, 512, 1, img);
            }
            while (JPEGstart(buffer) == false);

You never fread in that loop, meaning the condition will never be met, making the loop always execute exactly once.

There are a few ways to fix this. The way you seem to try it, with nested loops, usually results in more complicated code (assuming you want code that's correct, not just passing test cases). For example, your inner loop would have to stop at input file end, and your outer loop would have to stop there as well, meaning you test same thing twice, plus if you hit the next header sector, you might want to somehow inject it into the next iteration of the outer loop instead of freading the next sector.

The approach I prefer is having one while (not do..while) loop with the fread in the condition as you have it, and keep the open output file in a variable declared outside the loop (so I can access the value put there by some previous iteration). And inside the loop, I'd react on whether I have just found a header, and whether I already have an output file open (2 different choices each, 2x2=4 combinations, if you group them in a clever way that's not that much code).

Output files should begin with 000.jpg, not 001.jpg.