Pset 3 Recover : segmentation fault for “already found JPEG” loop

Question

I'm having some difficulties understanding the flow of recover. I've used the code and have written the pseudocode like this:
if the start of a JPEG file is found
- if it is first JPEG, create new file to store
- else (not the first), close previous file, create new file to store
else (after 512 bytes and no new JPEG file is found)
- if already found JPEG, then all the bytes belong to the JPEG file
- else (no JPEG file yet), repeat loop

I used debug50 on my code and found that the segmentation fault occurs at the "else (after 512 bytes and no new JPEG file is found)" loop. I am wondering whether my understanding of the process is wrong, or there's something wrong with my code.

The part which I think caused the error is this:

//Still no JPEG after 512 bytes, check if already had a previous JPEG
        else
        {
            //if yes, then it belongs to the currently opened JPEG file
            if (firstJPEG == false)
            {
                fclose(img); //close the image as you already found the JPEG
                jpegFiles++; //increase the count of the number of JPEG files found

                sprintf(filename, "%03i.jpg", jpegFiles);
                img = fopen(filename, "w");
                fwrite(&buffer, 512, 1, img);
            }

            //if no, continue to read through to find a new JPEG file or go back to loop
            else
            {
                fwrite(&buffer, 512, 1, img);
            }
        }

Here is my full code:

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

typedef uint8_t  BYTE;

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        fprintf(stderr, "Please enter a valid file\n");
        return 1;
    }
    //Open the card file, make sure the input is valid
    //Enter user input (file name) to open file
    FILE *inptr = fopen("card.raw", "r");

    //if the file is not working
    if (inptr == NULL)
    {
        fclose(inptr);
        fprintf(stderr, "Invalid file %s.\n", argv[1]);
        return 2;
    }
    //create buffer struct to hold 512 bytes
    BYTE buffer[512];
    //create a file to store output
    FILE *img = NULL;
    char filename[8]; //need 8 characters, because file name will be 000.jpg and one more for a null terminator
    //number of JPEG files found
    int jpegFiles = 0;
    //to indicate whether the first JPEG has been found or not
    bool firstJPEG = true;



    //Read through 512 blocks, 1 byte each, using a while loop, to check for a start of a JPEG
    while(fread (buffer, 512, 1, inptr) == 1)
    {
        //if yes for start of a JPEG
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            //if it is the first JPEG
            if (firstJPEG) // firstJPEG = true
                {
                    sprintf(filename, "%03i.jpg", jpegFiles);
                    img = fopen(filename, "w");
                    fwrite(&buffer, 512, 1, img);
                    jpegFiles++; //increase the count of the number of JPEG files found
                }
            //else means it is not the first JPEG, so we need to close the previous file
            else
                {
                    fclose(img); //close the image as you already found the JPEG
                    sprintf(filename, "%03i.jpg", jpegFiles);
                    img = fopen(filename, "w");
                    fwrite(&buffer, 512, 1, img);
                    jpegFiles++; //increase the count of the number of JPEG files found
                }

        }

        //Still no JPEG after 512 bytes, check if already had a previous JPEG
        else
        {
            //if yes, then it belongs to the currently opened JPEG file
            if (firstJPEG == false)
            {
                fclose(img); //close the image as you already found the JPEG
                jpegFiles++; //increase the count of the number of JPEG files found

                sprintf(filename, "%03i.jpg", jpegFiles);
                img = fopen(filename, "w");
                fwrite(&buffer, 512, 1, img);
            }

            //if no, continue to read through to find a new JPEG file or go back to loop
            else
            {
                fwrite(&buffer, 512, 1, img);
            }
        }


    }
    //close files
    fclose(inptr);
    fclose(img);
    return 0;

}

Thank you!

Answer 1

You're right, that code block is where the seg fault occurs, but perhaps not exactly where and why you think.

The seg fault is occurring when the very first block of data is processed. Let's think about the conditions at that point. No file has been opened and, basically, nothing has been done.

Now, the code drops through to that block of code. It looks at firstJPEG and sees that it is true, so it skips down to this code:

        else
        {
            fwrite(&buffer, 512, 1, img);
        }

This else clause is trying to write out the buffer into the "first file", but no output file has been opened yet. This causes a seg fault.

The idea of checking whether the first file has already been opened in order to decide whether to close it is good and necessary. However, it's also necessary to check whether the first file has been opened at all!

You're on the right track, and it'll work, but maybe there's a more efficient way to do this. Think about how this could be used: if(img != NULL)... or if(img == NULL).... While indicator variables are useful tools, (especially when the same condition would be tested in many places, or repeatedly in a loop where the condition wouldn't change) it's often better to go to the source, especially when the indicator would be used only at one or maybe two places. ;-)

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