CS50 Recover (Pset4): Segmentation fault-CodePudding

Basically title. I'm getting a segmentation fault instead of 50 jpeg images when I run my program.

I've tried playing around with different conditions.

Code:

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

const int BLOCK_SIZE = 512;
FILE *img = NULL;

int main(int argc, char *argv[])
{
    if (argc != 2) // Ensure correct usage of program
    {
        printf("Usage: ./recover IMAGE");
        return 1;
    }

    FILE *card = fopen(argv[1], "r");
    if (card == NULL) // Check if file was successfully opened
    {
        printf("Error! File cannot be opened");
        return 1;
    }

    typedef uint8_t block; // type for buffer
    block store[BLOCK_SIZE]; // Buffer for storing data from card.raw
    int count = 0; // Counter for file name
    char filename[8]; // Array to store filename

    while (fread(store, 1, BLOCK_SIZE, card) == BLOCK_SIZE)
    {

        if (store[0] == 0xff && store[1] == 0xd8 && store[2] == 0xff && ((store[3] & 0xf0) == 0xe0)) // If start of a new jpeg
        {
            if (count == 0)
            {
                count  ;
                sprintf(filename, "i.jpg", count); // Write filename with sequential number
                img = fopen(filename, "w"); // Open img file to write to
                fwrite(store, 1, BLOCK_SIZE, img);
            }
            else if (count > 0)
            {
                fclose(img);
                count  ;
                sprintf(filename, "i.jpg", count);
                img = fopen(filename, "w");
                fwrite(store, 1, BLOCK_SIZE, img);
            }
        }
        else
        {
            fwrite(store, 1, BLOCK_SIZE, img);
        }
    }
}

Edit: Sorry, messed up formatting the first time around. Code should be in now. Also, I'm now getting output that I can open, but the images all consist of a gray and white checkerboard pattern.

CodePudding user response：

In the Recover task of Problem Set 4 of CS50, the first two 512 byte blocks of the raw data do not yet contain the start of a JPEG file. Therefore, when running your program line by line in a debugger, you should observe the following:

When the program reaches the line

if (store[0] == 0xff && store[1] == 0xd8 && store[2] == 0xff && ((store[3] & 0xf0) == 0xe0))

for the first time, this condition will evaluate to false, because you have not yet reached the start of a JPEG file. Therefore, the else block will be executed instead, which contains the following line:

fwrite(store, 1, BLOCK_SIZE, img);

At this point, the debugger will show you that the value of img is still NULL. This means that you are passing an invalid FILE * to the function fwrite, which invokes undefined behavior and is probably the cause of the segmentation fault.

The simplest way to fix this would be to change the line

fwrite(store, 1, BLOCK_SIZE, img);

to:

if ( img != NULL )
{
    fwrite(store, 1, BLOCK_SIZE, img);
}

That way, the program will not attempt to write anything if a file has not yet been opened.

See this video link on how to run a debugger from CS50.

It is also worth noting that you have a significant amount of unnecessary code duplication in the following lines:

if (count == 0)
{
    count  ;
    sprintf(filename, "i.jpg", count); // Write filename with sequential number
    img = fopen(filename, "w"); // Open img file to write to
    fwrite(store, 1, BLOCK_SIZE, img);
}
else if (count > 0)
{
    fclose(img);
    count  ;
    sprintf(filename, "i.jpg", count);
    img = fopen(filename, "w");
    fwrite(store, 1, BLOCK_SIZE, img);
}

These lines can be simply rewritten to:

if ( img != NULL )
{
    fclose( img );
}

count  ;
sprintf(filename, "i.jpg", count);
img = fopen(filename, "w");
fwrite(store, 1, BLOCK_SIZE, img);

That way, you have no code duplication.