pset 4 Recover creates 50 corrupted or mainly grey images

Question

I wanted to try using malloc and a BUFFER structure of 512BYTES for this exercise with the intention of creating a file if the 4 signatures of a JPG are detected and fwriting the struct of 512B into the new file, fwriting the struct into a preexisting file if the 4 signatures of a JPG are not detected, or simply freeing the struct if a file does not exist. However, I just get 50 corrupted or grey images based on whether I use if (access( title, F_OK ) != -1) or if(outptr != NULL) (not used in my code at the moment) for my file existence checking, but no matter what no correct images. What am I doing wrong? Also, just a question regarding malloc and free. Everytime I free buf, I have to do buf = malloc(sizeof(BUFFER)) again which is why it is at the bottom of my loop else I get a double free error. Is it because once I free buf, both the allocated space of 512 BYTES and the 512 previously stored bytes are discarded meaning buf is back to pointing at nothing? What happens if I do not malloc again and I fread with buf? Where do the 512 read bytes go since I did not encounter a segmentation fault error but rather a double free error.

[code hidden]

Answer 1

First, the problem with the code lies in this line:

fwrite(&buf, sizeof(BUFFER), 1, outptr);

Adding the & is causing the problem. If you remove it, it will work. If you look at the man page for fwrite, you will see that the first parameter is supposed to be a pointer, not the contents of the memory pointed at, which is what the & triggers. (It's a little confusing. I often mess up with pointers myself because of stuff like this.)

Then, there's malloc and free. Understand that free() will release the memory pointed to by it's argument, but doesn't change the address stored in the argument. That means that when you free the memory, that memory is no longer owned or legally available to the program. So, when you want to use buf again, you have to call malloc() again to allocate new memory and to store that memory's address in buf. (It may or may not be the same physical memory and address.) A call to free() does not reset or reinitialize the contents of memory previously malloc'd. It simply releases it.

The double free or corruption is almost always a double free. This happens when you try to call free() on the same argument a second time without having re-malloc'd memory to that argument pointer. I'll leave it to you to trace your own logic and figure out why that is happening.

While your general concept of malloc'ing and freeing buf is valid, it isn't really necessary and is inefficient. It isn't that you are mallocing and freeing the memory, it's that you are trying to do it for every block read. Instead, you could malloc the memory for the buffer just once, before the while loop starts, and free it once after the while loop ends.