Segmentation fault pset4 recover.c

Question

I am stuck with a segmentation fault after finishing up the code for recover.c. I have read the code over a few times but it seems to make sense to me... So I can't really tell what I'm doing wrong. I have ran valgrind to see if there are any memory leaks. It shows that there might be some error on line 60, but I can't seem to find anything wrong there either. So I was wondering if someone can please help me look over my code and point out what I did wrong.

Thanks! :)

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

int main(int argc, char* argv[])
{
// open memory file
FILE* inptr = fopen("card.raw", "r");

//assigning stuff
char jpgname [8];
int c = 0;
uint8_t buffer[512];

if (inptr == NULL)
{
printf("Could not open file");
return 1;
}   

FILE * outptr = NULL;


while(fread(buffer, 512, 1, inptr) == 1)
    {

fread(buffer,512,1,inptr);

if( (buffer[0] == 0xff && buffer [1] == 0xd8 && buffer [2] == 0xff && buffer [3] == 0xe0 )|| (buffer[0] == 0xff && buffer [1] == 0xd8 && buffer [2] == 0xff && buffer [3] == 0xe1 ))

{

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

sprintf (jpgname, "%d.jpg", c);

FILE * outptr = fopen(jpgname, "w");

fwrite (&buffer,512,1,outptr);


fread(&buffer,512,1,inptr);
}

if( (buffer[0] != 0xff && buffer [1] != 0xd8 && buffer [2] != 0xff && buffer [3] != 0xe0 )|| (buffer[0] != 0xff && buffer [1] != 0xd8 && buffer [2] != 0xff && buffer [3] != 0xe1 ))

{
fwrite (&buffer,512,1,outptr);
}

fclose (outptr);
return 0;
}

}

Answer 1

You're getting a seg fault at the fclose(outptr); near the end of the program because it will attempt to close a file that has never been opened.

The while loop reads in a block, sees no signature, skips past all of the if statements and then hits the fclose(), having never opened an output file, and while outptr points to null. This might not be a problem if you moved the fclose and return to outside of the while loop.

After you fix this, you have a much more severe problem. You have back to back freads at the top of the while loop - the first in the while loop construct statement, and the second on the line immediately following, so you are discarding every other block of 512 bytes! The fread() call in the while statement isn't just a test to see if it's hit EOF, it is also an actual execution of fread()!

There are other issues remaining in your code, but I don't want to take all the fun of debugging away. I will say this though. It is possible to write this program with one fread() and one test for signatures. The more of each that are added to the program, the more complicated it gets and the more logic errors that tend to be added to the program. At least now you know where the seg fault comes from!

[EDIT: I was close, but not quite right.]

After looking more closely, the cause is pretty much the same, but the place where the seg fault isn't. As it turns out (I ran the code to verify), the seg fault occurs at the last fwrite() in the program. The first several thousand blocks of data do not have the signature, so the code drops down to the last if() statement, with a non-signature block of data and no output files having been opened. It will then pass this if() statement's test and the fwrite() will try to execute, but no output file was ever opened. This causes the seg fault. BTW, trying to close a file that was never opened, i.e., file pointer = NULL, will result in a seg fault too.

Kareem is absolutely right that the shadowed variable for the pointer is a big problem. It should only be declared in one place. Like I said earlier, there are other problems.

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