0

So my problem is that I get a segmentation fault when I run this code and I'm totally unsure why. I don't really understand what is happening in gdb either. This is definitely the most challenging thing I've done so far and am struggling a bit so help would be massively appreciated.

/**
* recover.c
*
* Computer Science 50
* Problem Set 4
*
* Recovers JPEGs from a forensic image.
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>

int main(int argc, char* argv[])
{
    // check there are no command line arguments
    if (argc != 1)
    {
        printf ("This programme does not accept command line arguments");
        return 1;
    }
    // Open card.raw
    FILE* filein = fopen("card.raw", "r");
    if (filein == NULL)
    {
        printf ("Could not open file");
        return 1;
    }

    typedef uint8_t BYTE;
    BYTE buffer[512];
    int i=0;
    char title[7];
    // read in first 512 bytes
    FILE* output=0;
    while (fread(buffer, 512, 1, filein)==1)
    {
        // check if buffer has either key jpg strings
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
        {
            // if this is not the first jpg file, then close the file
            if (i>0)
            {
                fclose(output);
            }
            // open up a new file and write 512 bytes to it
            sprintf (title,"%.3d.jpg",i);
            FILE* output = fopen(title, "w");
            fwrite (buffer, 512, 1, output);
            //increment i
            i++;
        }
        // if the block does not start with a jpg sig
        else
        {
            // check if a jpg is open, if yes - write to file, if no - do nothing
            if (i>0)
            {
                fwrite (buffer, 512, 1, output);
            }
        }
    }
    // close last file
    fclose(output);
}

Thank you!

7

Your problem is here:

while (fread(buffer, 512, 1, filein)==1)
{
    //if no jpg found and no jpg id then skip
    if (i == 0 && buffer[0] == 0 && buffer[1] == 0 && buffer[2] == 0 && buffer[3] == 0)
        continue;
    // check if buffer has either key jpg strings
    else if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
    {
        // if this is not the first jpg file, then close the output
        if (i>0)
        {
            fclose(output);
        }
        // open up a new file and write 512 bytes to it
        sprintf (title,"%.3d.jpg",i);
        printf("%.3d.jpg\n",i);
        FILE* output = fopen(title, "w");
        fwrite (buffer, 512, 1, output);
        //increment i
        i++;
    }
    // if i != 0 and buffer does not have the start of jpg signature, then write to current file
    else
    {
        fwrite (buffer, 512, 1, output);
    }
}

Let's consider what happens the first time this loop is run. First of all, even if you specified the buffer as BYTE buffer[512], that doesn't mean that it's going to be all zeroes too. To do that you should have done

BYTE buffer[512] = {0};

Although that is not the problem, apart from the point where you check for buffer[0] == 0 && buffer[1] == 0 && buffer[2] == 0 && buffer[3] == 0. You see on the top of the loop you have fread(buffer, 512, 1, filein) so by the time the loop runs for the first time you have already filled the buffer with the values read from the card.raw file. And nobody said that if this block of 512 bytes doesn't contain the beginning of a jpg, it's gonna be all zeros.

So this condition is never met, and effectively you don't pass the initial part of the card.raw that doesn't contain a jpg.

Then the flow goes to your else if statement, which will evaluate to false since there is no beginning of a jpg in the buffer, and so the else block will execute.

But since the else if block never execute, you didn't open a new file to write, and the fwrite in else tries to write at NULL giving you a segfault.

I'll leave it to you to figure out how to correct it. This should put you in the right direction.

Tip #1

If you want to initialize a pointer, better use NULL instead of 0. So you should have

FILE* output = NULL;

Tip #2

If you want to learn how to use the gdb take a look at the instructions here to get you started. http://www.unknownroad.com/rtfm/gdbtut/gdbtoc.html

Edit after changes in code

Ok, you are really really close now. You have just two mistakes and your code works.

Mistake #1

In the unchanged code, and in this one too, above your while loop, you have declared output like so (if you followed my advice you have initialized it to NULL too):

FILE* output = NULL;

But inside your while loop you have one more

FILE* output = fopen(title, "w");

Since there is the FILE* at the beginning, this means it's a new declaration and not an assignment. So the second deceleration will be the one used inside the if block, since it's where its scope is, and since it has precedence over the outer output variable. But then, when you get out of the if block and try to fclose(output), the outer output has precedence, and since it's NULL, you are trying to close NULL and you get the segfault.

To correct this, just remove the FILE* from the beginning of the inner output, to change it from a declaration to an assignment.

Mistake #2

Now this mistake is easier to solve yourself so I'm gonna leave it to you. Just to give you a hint, the title should be like so:

['0', '0', '0', '.', 'j', 'p', 'g', '\0']

See that it ends with a '\0' (i.e. null byte) because it's a string. Fix that and your sprintf() usage and you're golden.


If this answers your question please accept it by clicking the gray check-mark to the left, so that it becomes green. You can also vote it up by pressing the up arrow above the check-mark. And don't forget to keep coding!

| improve this answer | |
  • Hi Chris, thanks for the help, i've followed your advice on the initialization of the buffer and pointer. I've modified my code to what I think should make sense now, but I'm still getting the seg fault. Any more clues on whats going wrong? Thanks in advance – GCOTT94 Sep 7 '15 at 19:50
  • Check my edited answer. – ChrisG Sep 7 '15 at 21:41
  • 1
    Thanks so much Chris, my code works perfectly now! – GCOTT94 Sep 8 '15 at 9:19
  • Thank you both! – 祝一帆 Feb 19 '19 at 10:10

You must log in to answer this question.

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