1

I don't understand how the freeing is working. I know this happens when I try to free memory twice. However, this is really stumping me.

I've tried to post just the relevant parts of the code.

FILE* file = fopen(path, "r");
if (file == NULL)
{
    error(500);
    return;
}

// load file's content
BYTE* content;
size_t length;
if (load(file, &content, &length) == false)
{
    error(500);
    return;
}

This is the load fucntion

bool load(FILE* file, BYTE** content, size_t* length)
{
    printf("\nLOAD STARTED\n");
    content = NULL;
    BYTE *data = NULL;
    int size = 0;
    while(!feof(file))
    {
        char ch = fgetc(file);
        size += 1;
        data = realloc(data, sizeof(BYTE) * (size));
        *(data + (size - 1)) = ch;
    }
    content = &data;
    *length = size;
    printf("\nLOAD ENDED\n");
    return true;
}

A little while later I'm calling free()

printf("\nFREEING CONTENT\n");
// free file's content
free(content);
printf("\nCONTENT FREED\n");

The printf statement FREEING CONTENT works after which I get the

munmap_chunk(): invalid pointer error.

Jens
  • 69,818
  • 15
  • 125
  • 179
abhi
  • 431
  • 5
  • 11
  • 1
    `content = &data;` -->> `*content = data;` – wildplasser Apr 16 '16 at 12:59
  • `data[size] = ch;` --> `data[size++] = ch;` – BLUEPIXY Apr 16 '16 at 13:02
  • 3
    @BLUEPIXY got the problem - you're never changing `size` so your `data` buffer never gets bigger. Also, [`while(!feof(file))` is (almost always) wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) - and it *is* wrong in your code. – Andrew Henle Apr 16 '16 at 13:10
  • @AndrewHenle Yeah I noticed that about size. After fixing it, I still get a segmentation fault while freeing. There is something wrong in the memory allocation. Although printing out *data like a string has all the contents of the file. – abhi Apr 16 '16 at 13:20
  • You never check the `realloc` return value for `NULL`. Why do you expect it can't fail? – Jens Apr 16 '16 at 13:23
  • And `content = NULL;` at the beginning is dangerous bullshit. `*content = NULL;` could work, but is not necessary (since you still have `data = NULL;` at the beginning. And (intend to) do `*content = data;` at the end.) – wildplasser Apr 16 '16 at 13:33
  • @AbhishekManiyal *Although printing out *data like a string has all the contents of the file.* Well, you do read and store the data, since`realloc()` does return a pointer to a zero-byte block of memory. But `read()` overruns the end of that buffer and overwrites other memory. This is a perfect example of "undefined behavior" in that it seems to work, but other problems happen later. – Andrew Henle Apr 16 '16 at 13:35

2 Answers2

0

This is the problem:

content = &data;

It assigns the address of a local variable that goes out of scope after the function returns. Since content is a function parameter, nothing gets written to where it points. Did you mean to write

*content = data;

instead? If so you, you should not set content = NULL because you want to use the address passed by the call load(file, &content, &length).

Jens
  • 69,818
  • 15
  • 125
  • 179
0

I managed to fix it. content was basically just a char*. So, I tried this and it worked...

I changed content = &data to content = &(data[0])

It works. Appreciate all the inputs. :D

abhi
  • 431
  • 5
  • 11