0

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!

0

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. ;-)

  • Hi! Thanks for replying - I managed to solve it after understanding why the code jumped straight to the block of code you highlighted. However, I'm unclear on the usage of if(img == NULL)... I managed to make my code work by using if (firstJPEG == 0) and else if (firstJPEG == 1) instead. Hope you can shed some light on why would using if(img == NULL) be more efficient? Thanks :) – freshwound Feb 26 '19 at 8:48
  • Simple. First, img is correctly set to NULL initially so that the pointer contains a NULL pointer and not just garbage. It should also not immediately open a file until one is needed. This is all correct. Now, the code uses an indicator value, firstJPEG. That variable has to be created and initialized. Then, it has to be updated when the first file is created and opened. It has to be tested in an IF statement when deciding anything. In the meantime, img is also created as a pointer, initialized to NULL, and updated when a new file is opened. (contimues...) – Cliff B Feb 26 '19 at 12:07
  • Instead of adding all the code that handles firstJPEG, ask yourself what the real condition is that means I should wirte the data out to a file? It is more efficient to eliminate the indicator value completely and use the following line: if( img != NULL) { //write the data block to the output file } If it is null, the first file hasn't been opened. If it isn't null, then the garbage data at the front of the file has been processed and now, all data needs to be written to output files. Same thing is accomplished without complicating it further with an extra var. (cont. 2....) – Cliff B Feb 26 '19 at 12:20
  • (...cont'd 2.) Eliminating the indicator var saves memory and a couple of processing steps in the code. It may seem like it's not much difference and doesn't matter. It probably doesn't here, but it's a good habit to always make code more efficient. Imagine if this was code deployed to a high volume production environment and runs millions of times a day. How wasteful would that be? Extra steps, or steps in the wrong order can cause a program to take a lot more time and memory to run! Remember the crack pset? Efficient code ran in under 2 mins. Inefficient code ran, but took hours. – Cliff B Feb 26 '19 at 12:28
  • That makes sense, thanks Cliff B! I will try to look at my code again and work through it with the if(img != NULL) method!! :) – freshwound Feb 27 '19 at 8:54

You must log in to answer this question.

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