so I have another newbie question for you. This function is meant to read all the bytes from a passed in file, store them in the heap and then store the address to those bytes in the passed in 'content' parameter and the length in the passed in 'length' parameter.
bool load(FILE* file, BYTE** content, size_t* length)
{
if (file == NULL)
{
return false;
}
//array to hold the bytes??
BYTE* buffer = malloc(sizeof(BYTE));
//To hold the number of bytes currently loaded
size_t size = 0;
//Pointer to keep track of where I am adding the data to in buffer
//get all the bytes from the file and put them in the buffer
for(int i = fgetc(file); i != EOF; i = fgetc(file))
{
buffer[size] = (char) i;
size++;
buffer = realloc(buffer, size + 1);
}
//dereference length
*length = size;
//derefernce content
*content = buffer;
//free(buffer);
return true;
}
So previously the larger program that this function was a part of did not work, yet when I commented out the
free(buffer);
call at the bottom my program started to work perfectly. I was motivated to comment this out when I came across a double free error. So my question is: why does calling free in this instance cause an error?
My intuition tells me that it is because the data that
*content
points to is now "deleted" and so my program wasn't working. Furthermore somewhere later on in the code I free content* as well, which is where the double free error comes from. However for some reason I am inclined to believe that the data is not actually "deleted".
Sorry if this is a lot, I have been confused by memory allocation, free, and pointers for a while and am trying to gain a deeper understanding.
When allocating memory (or resources in general), you must make sure that you have clear ownership semantics. Who owns the allocated resource and is responsible for freeing it?
For a function that allocate resources, sane semantics are that the function returns the allocated resource if successful, and unless otherwise specified, the caller owns that resource. If the function fails, the caller should not have to perform any cleanup.
Your load() function allocates a buffer. Throughout most of its function body, load() owns that buffer. Just before it returns success, it effectively transfers ownership of that buffer to the caller (via the content output argument). If load() had a failure point after allocating the buffer, then it should call free(buffer) along that failure path.
I also feel compelled to point out a few issues with your code:
BYTE* buffer = malloc(sizeof(BYTE));
You should test if malloc fails. Additionally, sizeof (BYTE) is not useful since it is, by definition, 1.
buffer = realloc(buffer, size + 1);
This is poor practice. If realloc fails, you will lose the old value of buffer and will leak memory. It's better to do:
BYTE* tmp = realloc(buffer, size + 1);
if (tmp == NULL)
{
free(buffer);
return false;
}
buffer = tmp;
Finally, growing your buffer by 1 byte every time is very inefficient. More typical practices would be to double the buffer size or to grow it by a larger amounts.