1

For recover.c my program compiles but when I run gdb I get a segmentation fault and I'm not quite sure what it means.segmentation fault

When I run gdb I reach line 59 before I get this error message. I would like to know where the error actually occurs and what it means.

My code:

/**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 5
 *
 * Recovers JPEGs from a forensic image.
 */

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

#define BLOCK 512

typedef uint8_t BYTE;

int main(void)
{
BYTE jpeg[512];
BYTE buffer[BLOCK];
char title[8];
int pcount = 0;
FILE* img = NULL;

//open card file
FILE* file = fopen("card.raw","r");
if(file == NULL)
{
    printf("Could not open memory card.");
}

    printf("%d",sizeof(buffer));
    printf("%d",sizeof(BLOCK));
    printf("%d",fread(&buffer, sizeof(buffer),1,file));

//repeat until end of card
while ((fread(&buffer, sizeof(buffer),1,file)) == 1)
{

    //if start of a new JPG
    if (jpeg[0] == 0xff && jpeg[1] == 0xd8 && jpeg[2] == 0xff && (jpeg[3] == 0xe0 || jpeg[3] == 0xe1) )
    {
            //if image has not been opened
            if(img == NULL)
            {
                sprintf(title,"%3d.jpg",pcount);
                img = fopen(title,"a");
                fwrite(&buffer, sizeof(buffer), 1, img);
                pcount++;
            }
            //if image has  already been opened
            else if(img!= NULL)
            {
                fclose(img);
            }            
    }

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

}
fclose(img);
fclose(file);
}
3
  • Usually, a 'segfault' means your code is trying to access some memory address it's not allowed to. But it's hard to know why this is happening without actually looking at your source code. Try posting it here, and let others have a look at it. Four eyes see more than just two ;) – abelinux Nov 27 '14 at 19:39
  • In case it helps, most of the times segfaults are associated with wrong array indexing, or bad pointer accessing. – abelinux Nov 27 '14 at 19:41
  • I've added my code @abelinux – Kim Nov 27 '14 at 20:09
0

I'm not sure which is your line #59 (I only get #55 pasting your code in my editor ;).

I found some details:

  • On line #17 (my line #17) you wrote:

    if(file == NULL)`
    

    There's an extra '`' after the brackets. (probably it's not there in your code: it would not even compile it)

  • On line #25:

    while ((fread(&buffer, sizeof(BLOCK),1,file)) == 1)
    

    I'm pretty sure that's what's causing the 'segfault', though indirectly (we'll come back to that later:). Besides, I'm pretty sure it's not what you're trying to do. 'BLOCK' is a constant (an 'int', actually) and as such, is the size of an int ( = 4, on my system). So you're actually telling your system to read in 1 item at a time, of size 4 bytes, and store it into 'buffer'.

    What you were (probably) trying to do is:

    fread(&buffer, sizeof(buffer),1,file)
    

    (try printing out the size of all those variables/constants)

  • Lines #28 and #29:

    //read 512 bytes into a buffer
    fread(&buffer, sizeof(BLOCK),1,file);
    

    Inside the loop, you read in some bytes again. 'C' will actually read in twice every time through your loop. So, you'll be overwriting 'buffer' with new information, and loosing the bytes already read in when checking the while condition.

  • So, here's the deal:

    When you enter the 'while' loop for the first time, 'C'checks the condition:

    fread(&buffer, sizeof(BLOCK),1,file)) == 1
    

    According to it's man page:

    On success, fread() and fwrite() return the number of items read or written. This number equals the number of bytes transferred only when size is 1.

    So, since you're reading in 4 bytes (as already stated), the function returns = 4. That means 'C' compares:

    4 == 1  --> false
    

    It will never enter the loop.

    Therefore, when it gets to line #53:

    fclose(img);
    

    it will try to close a file that's never been opened. According to it's man page:

    The behaviour of fclose() is undefined if the stream parameter is an illegal pointer, or is a descriptor already passed to a previous invocation of fclose().

    I'm not totally sure, but pretty confident that passing in a 'NULL' pointer is really close to passing in "an illegal pointer".

HTH!

P.S.: you should always try inserting 'printf()' statements inside your code, bisecting it, and try to reach a conclusion as to which is the line producing the error. In this case, you would've been able to see that you are never entering the while loop, and that could've been a guide as on where to start the bug hunting ;)


EDIT:

I made a few modifications on your new version (v2) of your code:

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

#define BLOCK 512

typedef uint8_t BYTE;

int main(void)
{
    printf("%s\n", "Test1");

    BYTE jpeg[512];
    BYTE buffer[BLOCK];
    char title[8];
    int pcount = 0;
    FILE* img = NULL;

    int read;   /* edit */

    printf("%s\n", "Test2");

    //open card file
    FILE* file = fopen("card.raw","r");

    printf("%s\n", "Test3");

    if(file == NULL)
    {
        printf("Could not open memory card.\n");
    }

    printf("%s\n", "Test4");

    printf("%ld\n",sizeof(buffer));

    printf("%s\n", "Test5");

    printf("%ld\n",sizeof(BLOCK));

    printf("%s\n", "Test6");

    read = fread(&buffer, sizeof(buffer),1,file);
    printf("%ld\n", read);

    printf("%s\n", "Test7");

    //repeat until end of card
    while ((fread(&buffer, sizeof(buffer),1,file)) == 1)
    {
        printf("%s\n", "INSIDE WHILE: Test8");

        //if start of a new JPG
        if (jpeg[0] == 0xff && jpeg[1] == 0xd8 && jpeg[2] == 0xff && (jpeg[3] == 0xe0 || jpeg[3] == 0xe1) )
        {
                //if image has not been opened
                if(img == NULL)
                {
                    sprintf(title,"%3d.jpg",pcount);
                    img = fopen(title,"a");
                    fwrite(&buffer, sizeof(buffer), 1, img);
                    pcount++;
                }
                //if image has  already been opened
                else if(img!= NULL)
                {
                    fclose(img);
                }            
        }

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

    }
    fclose(img);
    fclose(file);
}

Try it yourself, and see if you can spot something. First thing you'll notice is that printf statements do print out prior to the segfault.

A couple more hints I'll give you: when you fread() or fwrite() from/to a *FILE stream, 'C' is responsible for "moving the cursor" around it. All you need to do is "call" it repeteadly, and the function will return sequential chunks of data.

There is, though, one precaution you should take: when you fopen() a file, the function loads the data into a memory address you passed in as an argument (that would be buffer). When you repeteadly keep calling fopen() (in the while loop) the cursor keeps moving forward in memory.

But, according to fread()'s man page:

If an error occurs, or the end of the file is reached, the return value is a short item count (or zero). fread() does not distinguish between end-of-file and error, and callers must use feof(3) and ferror(3) to determine which occurred.

So, you should either:

  • add a new feof(file) condition to your while loop

  • test the last fread() returned value, after leaving the while loop, to confirm whether you crushed into a read error, or you actually finished reading your file successfully.

This "auto-moving-the-cursor" has another consequence: if, like you did on your v2 code (I changed it) line #35, print out the result of fread():

printf("%d",fread(&buffer, sizeof(buffer),1,file));

prior to your actual read procedure (in your case, it would actually start with the while loop), the cursor will not be at the position you intend it to be (i.e.: at the very beginning of you file) when you actually want to start reading in the info.

There's more to review in your code, but I'm hoping I've convinced you to start printing out values (and even dummy lines) to be able to identify problems more accurately.

I was told by a teacher once:

Don't ever try to guess/find why your code don't do what it's supposed to do. That's, most of the time, a loss of time. Instead, always try to find out and understand why it does what it does. That's almost always a better starting point for a bug-hunting process.

Try it out and see if you get any closer! :)

(of course, come back and I'll be happy to help again ;)

8
  • I used print in gdb to print my values for sizeof(buffer) and my fread statement (because my program doesn't even run so printf doesn't work) so I understand why my loop was never entered. I believe the problem lies with the initialization of img. I understand that NULL doesn't work but wouldn't I have to give img a value before I begin to use it? – Kim Nov 29 '14 at 18:19
  • "because my program doesn't even run so printf doesn't work" --> not true! if your program compiles, but throws a segfault when executing, you can insert printf() statements inside your code (and recompile) so you can find exactly in which line the segfault is produced: all the printf statements previous to the "guilty" line will actually print out. Of course, you can achieve the same result using 'gdb' (that's precisely the purpose it serves;) – abelinux Nov 29 '14 at 21:23
  • well.. yes and no.. On (my) line #11 you write FILE* img = NULL;: you're declaring img. Then, on line #40 img = fopen(title,"a");: there's your initialization. The problem lies in you attempting (on line #38) fclose(img); even on your first attempt, when img is pointing nowhere yet. As @curiouskiwi stated [here] (cs50.stackexchange.com/a/4645/1740), you should check such condition before you attempt to "close the file" (or, more properly, the stream) – abelinux Nov 29 '14 at 21:33
  • The printf statements do not work for me at all. If I try to run the program the segfault pops up one time and when I use gdb it just shows the line of code and not the actual value that I want to see printed. Also, I have tested the condition but now I get an infinite loop and another segfault. – Kim Dec 1 '14 at 15:20
  • Okay so I've updated my code to show what I have now. I believe my problem lies in the else condition. This condition is supposed to keep reading in the blocks of data BUT if no pictures on the card have been found yet(before the first picture had been found) the program is just going to go straight to that else condition but img is still NULL at that point and therefore I get a segfault. However when i try to rectify this using else if( img != NULL) I get an infinite loop and another segfault. – Kim Dec 1 '14 at 16:12
0

You are reading the buffer in twice and only dealing with it once. That isn't causing your segfault but it will give you problems:

while ((fread(&buffer, sizeof(BLOCK),1,file)) == 1)
{

//read 512 bytes into a buffer
    fread(&buffer, sizeof(BLOCK),1,file);

Also, I don't think you can fclose(img) if img is NULL so you'll want to test that condition first.

1
  • Wouldn't I have to initialise img to something before I use it in my loop? – Kim Nov 29 '14 at 18:14

You must log in to answer this question.

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