0

I cannot seem to get to get rid of this seg fault. In my loop I basically have this (semi-pseudo code):

fread(in);
    if(nf == 0xffd8ffe0 || nf == 0xffd8ffe1)
    {
        f++;
        fclose(out);
        sprintf(...); //still working on this function
        filer = fopen(out);  
    }
    fwrite(out);
    fread(in);
    fwrite(out);
 ...

EDIT 0: valgrind returned the following:

 Invalid read of size 4
 ==13569==    at 0x41917E67: fread (iofread.c:41)
 ==13569==    by 0x80486BE: main (recover.c:36)
 ==13569==  Address 0x774b3439 is not stack'd, malloc'd or (recently) free'd

EDIT 1: ok so here is my new code (I still seem to be getting errors)

    for(int i = 1; i < 300; i++)
{
    buffer = malloc(512); // this was declared as a long long*
    fread(&nf, 4, 1, file);
    fseek(file, -4, SEEK_CUR);
    if(nf == 0xffd8ffe0|| nf ==0xffd8ffe1)
    {

        fclose(filer);
        char* g = "00";
        if(f > 10)
        {
        g = "0";
        }
        else if(f > 100)
        {
        g = "";
        }
        sprintf(fn, "%c%i", *g, f);
        filer = fopen(fn, "w");

    }
    fread(&buffer, 512, 1, file);
    fwrite(&buffer, 512, 1, filer);
    printf("iteration %i\n", i);
    free(buffer);
}

Edit 2: after adding in the naming system and revising my system, I no longer get the fault but I now get stuck in the first do_while loop. I can't seem to find the tags the designate the start of a new jpg. I have even looked in the file using xxd and can't seem to find it. I have also tried reunzipping the pset5 and taking a new card.raw to see if the first somehow got corrupted. sadly I had no luck with this either. here is my revised code:

 uint32_t nf = 0x00000000;    
char* fn = "0"; //filename
int f = 0;
FILE* filer = fopen(fn, "w");
int* buffer = malloc(512);
do
{
    fread(&nf, 4, 1, file);
    printf("%u", nf); //should be 4292411360 or 4292411361

}
while(nf != 0xffd8ffe0 && nf != 0xffd8ffe1);
fseek(file, -3, SEEK_CUR);

EDIT 3:

I have gotten rid of nf and now just check the first 4 values of buffer. Here is how my variable list looks:

 char* fn = "000"; //filename
 int f = 0;
 FILE* filer = fopen(fn, "w");
 int* buffer = malloc(sizeof(int) *512);

valgrind returns

 Conditional jump or move depends on uninitialised value(s)

when I try to compare the first few values in this format:

 if((buffer[0] == 0xFF) && (buffer[1] == 0xD8) && (buffer[2] == 0xFF) && (buffer[3] == 0xE0))

and then seg faults again. Im unsure if initializing and malloc'ing buffer correctly and any insight would be fo helpful

6
  • 1
    Have you tried running this in gdb or valgrind to narrow down the possible location of the error? Does it crash on the sprintf statement? Could it be related to the buffer which sprintf outputs to? – Luke Van In Aug 10 '14 at 23:22
  • @luke I have tried with gdb and ill try with valgrind – user1475 Aug 10 '14 at 23:24
  • Use GDB to check the value of nf in your while loop. I don't think your nf evaluation logic is doing what you think it is. – lethaljd Aug 14 '14 at 21:02
  • I'm printing nf already – user1475 Aug 14 '14 at 21:25
  • 1
    I would like to help you further, but the code in the question is not clear. Is it possible to to reduce the code to the part which is broken. The second block does not seem to be relevant, has it been replaced by the third code block, or are both being used, this is unclear? The third block does not seem to be internally consistent, e.g. it declares FILE* filer, but then accesses fread(&nf, 4, 1, file);. Did you mean to use filer or file? If the problem is just in the while loop, please only show that. Please be mindful of the honor code. – Luke Van In Aug 15 '14 at 21:36
2

The error from valgrind is showing you where the crash is occurring, and what the potential cause could be. Here's how to interpret it:

Invalid read of size 4

This means the program tried to access 4 bytes of memory. With experience you will recognise that this something to do with a pointer, since pointers are usually 4 bytes.

 ==13569==    at 0x41917E67: fread (iofread.c:41)

This is telling you the actual error occurred inside the fread function in iofread.c. This is part of the C standard library, which we assume to be correct. Or at less likely to contain bugs than our own code, so we keep looking.

==13569==    by 0x80486BE: main (recover.c:36)

This is the last line that was executed in our program before the crash. It's telling you the crash occurred in the main function, in recover.c, on line 36. Looking at the previous error we can assume this line is calling fread.

==13569==  Address 0x774b3439 is not stack'd, malloc'd or (recently) free'd

This line tells you what the potential cause of the error is. You'll need some intuition to put it into context. The error is telling you that fread tried to access a pointer that basically doesn't point to anything, because it isn't initialised at this point. This might be the pointer for the output buffer, or it could be the file handle pointer. The code just defined the pointer but never made it point to anything. The play-doh man has exploded.

Check that the file is fopen'd, and that the output buffer is malloc'd.


One way to check that card.raw does contain JPEG data is to use the grep to search in the output from xxd:

xxd card.raw | grep ffd8

This should show you output like the following:

enter image description here

Note the signature text is highlighted in red, and the column on the right shows the words JFIF (which incedentally stands for JPEG File Interchange Format). If you don't see this, then it means card.raw is not correct and your code will fail to extract any images.


Note some of the advice in the specification:

The implication is that these cameras only write to those cards in units of 512 Bytes

And...:

Moreover, rather than read my CF card’s bytes one at a time, you can read 512 of them at a time into a buffer for efficiency’s sake

And lastly:

Thanks to FAT, you can trust that JPEGs' signatures will be "block-aligned." That is, you need only look for those signatures in a block’s first four bytes.

This means that the program should read the file in blocks of 512 bytes at a time. You only need to inspect the first four bytes of the block to determine whether or not it is a JPEG.

Compare this to your code, where you are reading the file 4 bytes at a time. Not only is this unnecessary, it could lead to false positives where some content within the file is erroneously detected as the start of a new JPEG.


Some general tips:

  1. To print a value as HEX:

     printf("%08x", nf);
    
  2. Use sizeof whenever accessing memory, such as when reading from a file, or allocating memory for a pointer. For example, instead of this:

    int* buffer = malloc(512);
    

    Do this:

    int* buffer = malloc(sizeof(int) * 512);
    

    While the first method does produce a result, it is probably not doing what you intended. A problem arises when you try to access the allocated memory. Suppose you were to access the buffer using buffer[511]. Since buffer is a pointer to an int, this will try to access the 511th int in the array, which is something like the 2044th byte. In the first method you are only allocating 512 bytes, so when you try to access it as an int, you end up referring to some memory way outside the block that was actually allocated.

  3. For maximum platform compatibility, look for the individual byte values, instead of trying to compare the entire 4-byte int at once. For example, instead of this:

    int32_t b[128];
    b == 0xffd8ffe0
    

    Do this:

    char b[512]
    (b[0] == 0xFF) && (b[1] == 0xD8) && (b[2] == 0xFF) && (b[3] == 0xE0)
    

    The reason for this is that some CPUs store multi-byte values in a different order, depending on their endianness.

5
  • Im still getting the same errors after using malloc – user1475 Aug 11 '14 at 2:26
  • thank you so much for all the help. for some reason using if(b[0] == 0xFF) && (b[1] == 0xD8) && (b[2] == 0xFF) && (b[3] == 0xE0) returns "Conditional jump or move depends on uninitialised value(s)" (in valgrind) and I end up with another segmentation fault – user1475 Aug 16 '14 at 1:46
  • 1
    That means that b is a pointer and has not been initialized with a value. If b is a pointer then it needs to be malloc'd. – Luke Van In Aug 16 '14 at 10:46
  • 1
    This answer addresses similar concerns: cs50.stackexchange.com/a/2310/1707 – Luke Van In Aug 16 '14 at 12:01
  • if you mean char for b, wouldn't it be: (b[0] == "0xFF"), etc. – user1475 Aug 17 '14 at 22:25

You must log in to answer this question.

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