4

I have a data structure that is defined as follows:

struct varr {
    int n; //length of data array
    double data[];
};

The data array is required to be initially of size 1 but allowing the possibility of increase.

When allocating space for a struct varr * I use

struct varr *p = malloc(sizeof(struct varr) + sizeof(double));

When reallocating space to increase the size of the data array I use

p = realloc(p, sizeof(struct varr) + p->n * sizeof(double));//p->n having already been set

My question is "how should I free the memory allocated for this structure?"

I've tried a simple free(p); but this causes memory leaks according to memcheck. Is there something fundamentally wrong with how I'm structuring my data for this purpose, or with how I'm allocating the memory?

==NOTE==

I've solved the problem by using a pointer instead of an explicitly declared array. I'd still be interested in a concise answer for why this doesn't work, however.

Patrick White
  • 671
  • 5
  • 19
  • @KerrekSB Sorry - that's a typo. I'm not actually using sizeof(sizeof()). I'll edit. – Patrick White Nov 15 '12 at 04:36
  • 1
    What is the relation between `struct poly` and `struct varr`? – Keith Randall Nov 15 '12 at 04:37
  • `varr::n` should be a `size_t`. – Kerrek SB Nov 15 '12 at 04:38
  • @KeithRandall: Also a typo. KerrekSB true - but this is C, not C++ – Patrick White Nov 15 '12 at 04:39
  • 1
    @PWhite: You must be doing something else wrong if you're leaking memory (such as clobbering `p` if `realloc` fails as Kerreb SB pointed out) . You don't need to do anything special to deallocate memory that involves a flexible array member. `malloc`/`free` are completely agnostic to what you're using the memory for. – jamesdlin Nov 15 '12 at 06:08
  • @jamesdlin Whenever I realloc `p` I check it and immediately terminate if it's `NULL` since it's very important that I know about realloc being unable to allocate memory. – Patrick White Nov 15 '12 at 06:14

3 Answers3

7

As you probably know, each call to malloc makes OS give you some memory and remember it's size and attributes. Thus if you call free, you can clear both an array or a pointer.

Examples:

char* array = malloc(16 * sizeof(char));
char* single = malloc(sizeof(char));

free(array);
free(single);

As you can see, you always get one free for one malloc. That is because the OS knows how many bytes you allocated, it doesn't care what type it is and how many instances of it were created. (note: this is why there is difference between delete and delete[] in C++ because the application needs to know what destructors to run, the control of cleanup is not left to the OS only...)

Taking from here, we can assume that if we allocated the struct as a one block using a single malloc, it can be freed using a single free call.

This example works for me without any leaks:

#include <stdlib.h>

typedef struct Array_t
{
    int Length;
    double Data[];
} Array;

Array* create_array(int length)
{
    Array* array = malloc(sizeof(Array) + length * sizeof(double));
    if (array != NULL)
        array->Length = length;

    return array;
}

void delete_array(Array* array)
{
    free(array);
}

int main()
{
    Array* array = create_array(100);
    if (array == NULL)
        return EXIT_FAILURE;

    for (int i = 0; i < array->Length; ++i)
    {
        array->Data[i] = 1.7 * (i + 3);
    }

    delete_array(array);

    return EXIT_SUCCESS;
}

Of course, if you get up to something more complex like John Findlay example with

struct SomeStruct
{
    int Size;
    int* ArrayOfPointers[];
}

you can still create this struct in one malloc, e.g.

// *s* contains an array of 14 int pointers (int*)
struct SomeStruct* s = malloc(sizeof(SomeStruct) + 14 * sizeof(int*));
s->Size = 14;

The core problem is though that int* ArrayOfPointers is an array of pointers, thus to initialize it properly, you also need to

// each of the *s*'s int pointers is actually a decayed array of 25 ints
for (int i = 0; i < s->Size; ++i)
    s->ArrayOfPointers[i] = malloc(25 * sizeof(int));

And when freeing:

for (int i = 0; i < s->Size; ++i)
    free(s->ArrayOfPointers[i]);

free(s);

But the point is that the structure with FAM is still freed in one free call. The loop is freeing the allocated pointer data, which is equivalent to freeing a dynamically allocated 2D array.

Zdeněk Jelínek
  • 2,611
  • 1
  • 17
  • 23
  • This post is nearly three years old. – Patrick White Mar 01 '15 at 13:32
  • But I came across this problem and didn't find a solution/explanation anywhere. – Zdeněk Jelínek Mar 01 '15 at 15:47
  • 1
    I think the initial problem boiled down to me being a fairly poor C programmer 3 years ago. But this is the answer that most clearly explains the problem and solution, and in the spirit of improving stack overflow as a community wiki I've switched the accepted answer to this. – Patrick White Mar 01 '15 at 15:52
3

That looks flat out wrong. I think it should go like this:

// step 1: Allocate n items:

struct varr * p = malloc(sizeof *p + n * sizeof(double));
if (p) { p->n = n; }

// step 2: Reallocate to hold m items:

struct varr * tmp = realloc(p, sizeof *tmp + m * sizeof(double));
if (tmp) { p = tmp;  p->n = m; }

When done, don't forget to say free(p);.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • This causes my program to break - give me a minute and I'll give you more information. – Patrick White Nov 15 '12 at 04:48
  • Sorry "a minute" turned into an hour - odd bugs seemingly unrelated to this code appeared. In any case, this didn't fix the problem. I'm having more memory leaks than before. – Patrick White Nov 15 '12 at 05:57
  • I'll accept your answer for now, even though it doesn't solve the problem, because it points out some things that the original code did wrong. – Patrick White Nov 15 '12 at 06:24
0

I ran into the same problem last night. After a couple of hours of googling I found this, pretty simple but clever answer in a tutorial:

void free_struct( THESTRUCT * ts )
{
// free all memory accoiated with our structure
if(ts){
    for(int i = 0; i<ts->count; i++)
        free(ts->str[i]);
    free(ts);
    }
}

http://www.johnfindlay.plus.com/lcc-win32/Tuts/FlexArrs.htm

AAAton
  • 1,463
  • 2
  • 12
  • 11