0

This is for CS50 PSET 4 recover.

I am struggling with identifying the segmentation fault that check50 shows. Upon running, it seems to generate the 50 images correctly without error, and valgrind does not show any errors. My estimation is that on the run of the last image found through my loop, the fwrite function will write the same as the previous iteration of the loop, as the if condition will not be satisfied.

I am aware my code is inefficient running through the image file 1 byte at a time.

Thank you for the help!!

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{

//ensure only one cl input
if (argc != 2)
{
printf("Usage: ./recover IMAGE\n");
return 1;
}

//open image in read-mode
FILE *input = fopen(argv[1], "r");
if (input == NULL)
{
printf("Could not open file.\n");
return 1;
}

//find length of image in bytes
fseek(input, 0, SEEK_END);
int size = ftell(input);
rewind(input);

//keep track of outputted jpeg files
int rcounter = 0;
int stop_position = 0;
BYTE buffer[size];

//read input into buffer
fread(buffer, sizeof(BYTE), (size / (sizeof(BYTE))), input);

for (int i = 0; i < (size - 511); i++)
{
        if (buffer[i] == 0xff && buffer[i + 1] == 0xd8 && buffer[i + 2] == 0xff && (buffer[i + 3] & 0xf0) == 0xe0)
        {
            char file_name[10];
            sprintf(file_name, "%03i.jpg", rcounter);

            //if conditions met, create file for writing
            FILE* output = fopen(file_name, "w");
            if (output == NULL)
            {
                printf("Could not create %s.\n", file_name);
                return 1;
            }

            //find start of next jpg
            for (int j = (i + 1); j < (size - 511); j++)
            {
                    if (buffer[j] == 0xff && buffer[j + 1] == 0xd8 && buffer[j + 2] == 0xff && (buffer[j + 3] & 0xf0) == 0xe0)
                        {
                            stop_position = j;
                            break;
                        }
            }

            //write to output until start of next jpeg
            fwrite(&buffer[i], sizeof(BYTE), (size - stop_position) / (sizeof(BYTE)), output);
            rcounter++;
            fclose(output);
        }
}
fclose(input);
}

1 Answer 1

2

I duplicated your experience. Sometimes this happens. I looked at your code and didn't see anything immediately to explain it. However, it's probably related to your code inefficiency. You should consider rewriting the program.

It's obvious that the code is looking for a signature anywhere in the input file. This is both unnecessary and a problem The problem spec says that a signature will appear at the beginning of each 512 byte code block, so it's only necessary to check the first 4 bytes.

Next, it's also possible that random data inside the block could legitimately appear and match a signature. In fact, there is a signature buried inside the input file to look for this. Were it not for the seg fault, it would have failed check50 because of this.

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

[EDIT] There's also this. Your code reads the entire input file into memory. If check50 is using an input file that's larger than the memory stack available to the program (it's a big file!), it likely will seg fault. (Thanks, @curiouskiwi, for pointing this out.)

As a practical matter, if this were a real-world program, there would be no way to know how large an input file can be. It's better to read in and process out one 512 byte data block at a time, rather than reading in the entire file.

1
  • Lovely. This was so helpful in helping me understand my code and this PSET.
    – jgoteam
    Jul 15 at 3:07

You must log in to answer this question.

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