Recover creates an unreadable image file and then segfaults

Question

I am pulling my hair out over recover.c in pset5. My code manages to generate 1 appropriately named (but unreadable) jpg and then fails with segmentation fault. I have tried to figure it out by looking at existing answers, but haven't managed so far.

Any hints greatly appreciated, I am posting pseudocode below, but let me know if actual code is needed.

open input file 

declare filename[8]
declare buffer of size BLOCK
initialize counter         
initialise outptr img

while not EOF
{
   read data into buffer

   if (beginning of JPEG)
    {
        close previous output file if exists
        create and open new output file    
        update counter
    }

    if (output file exists)
    {
        // write buffer data to output file, this is the line valgrind complains about
        fwrite(&buffer, BLOCK, 1, img)
    }
}
close remaining files
free memory

Here are the relevant lines from Valgrind:

Valgrind output: 
==25646== Invalid read of size 4
==25646==    at 0x43C12357: fwrite (iofwrite.c:41)
==25646==    by 0x804886E: main (recover-1.c:69)
==25646==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

---

Thanks to the answer below, I am now generating 49 JPGs. Turns out that my EOF probe in the initial while loop was the issue, I should have realized that fread() returns the number of items of the specified size successfully read - which is an Integer and can't be compared to NULL, so here's how I fixed this issue:

//original (not working) code: 
while (fread(buffer, BLOCK, 1, file) != NULL)

//new (working) code: 
while (fread(buffer, BLOCK, 1, file) == 1)

Note my program is still not working fully, but as was kindly pointed out, this warrants a new question.

Answer 1

Address 0x0 in Valgrind hints that the program may be trying to dereference a pointer that is NULL. Check your code where you assign to pointers and ask yourself if you've accounted for every instance where the pointer could potentially be assigned NULL.

It might help to do some post-mortem debugging with gdb and inspect the values of the arguments you are passing to fwrite – particularly img.

Also, you should be able to pass buffer as an argument without the & operator. Not necessarily related to your error but here's some reading:

• fwrite - C++ Reference
• How come an array's address is equal to its value in C?