Freeing memory for nested structures

A

Andreas Eibach

Hi again,

one of the other big woes I'm having...

typedef struct perBlockStru /* (structure) Long words per block
*/
{
unsigned long *lword[LASTLW];
} lwperBlockStru_t;

typedef struct Entity /* (structure) Holds the whole thing */
{
perBlockStru_t* block[LASTBLK];
} entityStru_t;

This is a - fairly simplified - structure I'm using.
Of course there are more elements, but they are irrelevant here.

Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned long));
...
}
}

At the VERY bottom, shortly before exiting the app, I *thought* I could just
do:

if (pE != NULL)
free(pE);

Preposterously, it sometimes does work, but sometimes I also get a GPF.
Hence I assume that I am required to go the "awkward" way by using another
loop
in the destruction routine.
Or is this supposed to work by simply using the free routine from the
structure's base address?

-Andreas
 
N

Nate Eldredge

Andreas Eibach said:
Hi again,

one of the other big woes I'm having...

typedef struct perBlockStru /* (structure) Long words per block
*/
{
unsigned long *lword[LASTLW];
} lwperBlockStru_t;

typedef struct Entity /* (structure) Holds the whole thing */
{
perBlockStru_t* block[LASTBLK];
} entityStru_t;

This is a - fairly simplified - structure I'm using.
Of course there are more elements, but they are irrelevant here.

Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)

pE = &entityStru_t , I assume.

By the way, if this is a Unix/POSIX system, AFAIK you are not supposed
to name things ending in _t ; they're reserved for the system.
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned long));


I presume this is not your actual code, and that you really have
malloc(sizeof(unsigned long) * n) to allocate an array of n unsigned
longs. Allocating just one seems kind of weird, I suspect it wouldn't
be what you'd want.
...
}
}

At the VERY bottom, shortly before exiting the app, I *thought* I could just
do:

if (pE != NULL)
free(pE);

No. You must pass to free() exactly those pointers that were returned
from malloc(), and nothing else. free() does not know about your more
elaborate data structure and cannot dig down into it to find the things
you malloc'ed.
Preposterously, it sometimes does work, but sometimes I also get a GPF.
Hence I assume that I am required to go the "awkward" way by using another
loop
in the destruction routine.

Right.

However, if this really is right before exiting, it is probably not
necessary to free this stuff at all, as any sane operating system will
take care of it for you. See http://c-faq.com/malloc/freeb4exit.html .
It might still be a good idea to do it for maintainability reasons,
depending on your situation.
Or is this supposed to work by simply using the free routine from the
structure's base address?

No.
 
J

jameskuyper

Andreas said:
Hi again,

one of the other big woes I'm having...

typedef struct perBlockStru /* (structure) Long words per block
*/
{
unsigned long *lword[LASTLW];
} lwperBlockStru_t;

typedef struct Entity /* (structure) Holds the whole thing */
{
perBlockStru_t* block[LASTBLK];
} entityStru_t;

This is a - fairly simplified - structure I'm using.
Of course there are more elements, but they are irrelevant here.

Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)

What you wrote is nonsense. I suspect that what you were trying to say
would be correctly expressed as:

entityStru_t *pE;
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned long));
...
}
}

At the VERY bottom, shortly before exiting the app, I *thought* I could just
do:

if (pE != NULL)
free(pE);

Preposterously, it sometimes does work, but sometimes I also get a GPF.


You don't show us the code whereby pE acquires a value, so it's hard
to tell whether or not you should be able to safely call free(pE).
However, if you don't want to leak memory, you had better free(pE-
block->lword[j]) for all relevant values of "i" and "j", before

free()ing pE itself.
Hence I assume that I am required to go the "awkward" way by using another
loop
in the destruction routine.
Or is this supposed to work by simply using the free routine from the
structure's base address?

Was the memory for the structure itself allocated using malloc(),
calloc() or realloc()? You show no such allocation, so there's no way
for us to be sure. If it was, it can be free()d.
 
A

Andreas Eibach

Malcolm McLean said:
Write a "kill" function.

void killentity_Stru_t(entity_Stru_t *e)
{
int i, ii;

if(e)
{
for(i=0;i<LASBLK;i++)
for(ii=0;ii<LASTLW;ii++)
free(e->block->word[ii]):
free(e);
}
}

I think I've got this right.


Indeed you got, yet when I asked, "do I have to go the awkward way", even
mentioning the word "loop" in the very same breath, I _had_ thought that the
readers will realize I do know *how* to do that :)
But if I hate something it's trial-and_error beyond some certain extent;
hence I was just reflecting upon what I would start coding next ...
It becomes boilerplate code after a while. More
modern languages with garbage collection automate the process for you, but
in C you have to free everything by hand.
Well, I do know it that far. But I did not know *how* deeply (i. e. whether
C would "know" about sub-structures and stuff simply by creating a complex
chain of storage addresses or so---in this case, the base address would have
been sufficient; but apparently it does not)
Think of it as a little chore.
Yes, thank you. So the loop thing has to do the trick then. :)

-Andreas
 
J

jameskuyper

Andreas Eibach wrote:
....
But if I hate something it's trial-and_error beyond some certain extent;

You should hate it a whole lot more than you apparently do. When we're
talking about programming languages, the space you explore by trial
and error has both an huge volume and a very high dimensionality. It's
easy to get thoroughly lost in that space, and very difficult to find
out anything useful that way; finding what you're actually looking for
is practically impossible by that method. Reading the documentation,
reading text books, and asking for advice is the right way to go about
it.

....
Well, I do know it that far. But I did not know *how* deeply (i. e. whether
C would "know" about sub-structures and stuff simply by creating a complex
chain of storage addresses or so---in this case, the base address would have
been sufficient; but apparently it does not)

It doesn't matter what C "knows". What free(expression) is supposed to
do, if "expression" is not a null pointer, is to release the memory
pointed at by "expression"; something it can do if "expression" has a
value that matches one returned by a previous call to malloc(),
calloc() or realloc(). The behavior is undefined if you pass it any
other pointer value. It free()s the memory that was allocated by that
previous call - nothing more, nothing less.
 
A

Andreas Eibach

Nate Eldredge said:
Andreas Eibach said:
Hi again,

one of the other big woes I'm having...

typedef struct perBlockStru /* (structure) Long words per block
*/
{
unsigned long *lword[LASTLW];
} lwperBlockStru_t;

typedef struct Entity /* (structure) Holds the whole thing */
{
perBlockStru_t* block[LASTBLK];
} entityStru_t;

This is a - fairly simplified - structure I'm using.
Of course there are more elements, but they are irrelevant here.

Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)

pE = &entityStru_t , I assume.

yep.

entityStru_t * pE was the declaration, I'm often confusing them when I have
to word them, but I seem to have made it correctly, as pE->... does indeed
"find" the structure members ;-)
By the way, if this is a Unix/POSIX system, AFAIK you are not supposed
to name things ending in _t ; they're reserved for the system.

Heheh, yes it's a habit.

for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned
long));

I presume this is not your actual code,


You're right about that it's shortened a lot.
and that you really have
malloc(sizeof(unsigned long) * n) to allocate an array of n unsigned
longs.

Eh? I have a LASTLW amount of longs, and LASTLW is a #define'd constant, e.
g. 64. I do not use "hard-coded" numbers, it's a habit (but a good one this
time, I guess :))
Allocating just one seems kind of weird, I suspect it wouldn't
be what you'd want.

But I'm allocating LASTLW ones in a loop, you did notice the for() loop,
didn't you?
Come to think of it, I could get rid of the for loop and use n times the
ULONGs in the malloc, if that is what you mean.
free() does not know about your more
elaborate data structure and cannot dig down into it to find the things
you malloc'ed.

Yes, that was my suspicion and the same what Malcolm already pointed out.
Thanks to the both of you.
However, if this really is right before exiting, it is probably not
necessary to free this stuff at all [...]
It might still be a good idea to do it for maintainability reasons,
depending on your situation.

I'd always do it. Period. Never "rely" on how well the garbage collection
works.
(except for JAVA, the GC of which I do fully trust)

-Andreas
 
J

jameskuyper

Andreas said:
Nate Eldredge said:
typedef struct Entity /* (structure) Holds the whole thing */
{
perBlockStru_t* block[LASTBLK];
} entityStru_t;

This is a - fairly simplified - structure I'm using.
Of course there are more elements, but they are irrelevant here.

Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)

pE = &entityStru_t , I assume.

yep.

That still doesn't work. entitStru_t is a typedef for "struct
Entity". 6.5.3.2p1 says "The operand of the unary & operator shall be
either a function designator, the result of a [] or unary * operator,
or an lvalue that designates an object that is not a bit-field and is
not declared with the register storage-class specifier." The type
"struct Entity" fits none of those categories.
entityStru_t * pE was the declaration, I'm often confusing them when I have
to word them, but I seem to have made it correctly, as pE->... does indeed
"find" the structure members ;-)

Yes, but the code you've decided to not bother posting leaves us still
in the dark as to how the value of 'pE' is set. This is a critical
point in deciding whether or not your call to free() should actually
work.
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned
long));

I presume this is not your actual code,


You're right about that it's shortened a lot.


It's not just shortened, it's wrong. As written, both the inner and
outer loops are infinite, unless LASTBLK or LASTLW happens to be 0, in
which case the relevant loop doesn't execute at all. Re-check your
loop conditions.

....
But I'm allocating LASTLW ones in a loop, you did notice the for() loop,
didn't you?

Come to think of it, I could get rid of the for loop and use n times the
ULONGs in the malloc, if that is what you mean.

Yes, that's precisely what you should be doing. There's usually a
small memory overhead, and a large time overhead, associated with
every call to malloc().
 
N

Nate Eldredge

Andreas Eibach said:
Nate Eldredge said:
Andreas Eibach said:
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned
long));

I presume this is not your actual code,


You're right about that it's shortened a lot.
and that you really have
malloc(sizeof(unsigned long) * n) to allocate an array of n unsigned
longs.

Eh? I have a LASTLW amount of longs, and LASTLW is a #define'd constant, e.
g. 64. I do not use "hard-coded" numbers, it's a habit (but a good one this
time, I guess :))
Allocating just one seems kind of weird, I suspect it wouldn't
be what you'd want.

But I'm allocating LASTLW ones in a loop, you did notice the for() loop,
didn't you?
Come to think of it, I could get rid of the for loop and use n times the
ULONGs in the malloc, if that is what you mean.


You have an array of LASTLW pointers, and you set each of them to point
to a single (malloc'ed) unsigned long. It would be easier just to have
an array of LASTLW unsigned longs.
free() does not know about your more
elaborate data structure and cannot dig down into it to find the things
you malloc'ed.

Yes, that was my suspicion and the same what Malcolm already pointed out.
Thanks to the both of you.
However, if this really is right before exiting, it is probably not
necessary to free this stuff at all [...]
It might still be a good idea to do it for maintainability reasons,
depending on your situation.

I'd always do it. Period. Never "rely" on how well the garbage collection
works.
(except for JAVA, the GC of which I do fully trust)

This is a different thing from garbage collection. Garbage collection
relies on the compiler keeping track of all the pointers you malloc'ed,
knowing when they're no longer referenced, and freeing the memory when
needed. What I'm talking about is the operating system reclaiming every
byte of memory that belonged to your program, whether it was obtained by
malloc() or contained program code, all at one blow. There are no
subtleties and no wondering about how efficient it is. So as far as the
behavior of the program is concerned, that means that calling free()
just before exiting is a waste of code space, CPU time, and programmer
time.

Possible reasons why you might want to do it anyway:

1. Practice
2. In case you someday make the program do something more after the
structure is no longer needed, or repeat its process many times, or be
encapsulated in a library
3. In case you are using a primitive, embedded, or ancient system that
doesn't have a proper OS
 
A

Andreas Eibach

Eric Sosman said:
Andreas Eibach wrote:
Far below, there's the obvious mem allocation happening:
(pE = *entityStru_t)
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)

These loops look pretty strange. I *really* wish people with
code problems would post the actual code they're having trouble
with, and not "sort of like" paraphrases.
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned
long));

Case in point: We can guess what the loops were supposed to be,
and we've been told that `pE' is a pointer to an entityStru_t, but
there's nothing to show us (1) how memory was allocated for `pE' to
point at


To cut a long story short: I simply forgot to post that.
No malloc()'ing at this point.

[*] entityStru_t e;
entityStru_t * pE = &e;

pE now points to the structure, and AFAIK not needs any malloc'ing here,
because line [*] has reserved mem through 'e' for the (naked) structure.
nor (2) how memory was allocated for each `pE->block'.

OK, granted.
Gladly I have not forgotten about it, but I've just realized that I do the
block and block->lword[j] allocations at different locations. This is
actually nonsense, as lengths are expected to be fixed and
once the structure is fully built up, it may stay in the program until it
exit()s.

unsigned int blnr;
for (blnr = 0; blnr < LASTBLK; blnr++)
{
pE->block[blnr] = (perBlockStru_t *) malloc (LASTLW *
sizeof(perBlockStru_t));
}
a lot of guesses about what the "..." might be concealing, but there's
no way for me to actually know what you've done wrong. Oh, sorry,
that last word is just a guess ...

I do dig programmers' humor. Oh yes, I do. :p

-Andreas
 
A

Andreas Eibach

jameskuyper said:
Yes, but the code you've decided to not bother posting leaves us still
in the dark as to how the value of 'pE' is set. This is a critical
point in deciding whether or not your call to free() should actually
work.
for (i 0; i = LASTBLK; i++)
{ ...
for (j= 0; j = LASTLW; j++)
{ ...
pE->block->lword[j] = malloc (sizeof(unsigned long));

I presume this is not your actual code,


You're right about that it's shortened a lot.


It's not just shortened, it's wrong. As written, both the inner and
outer loops are infinite, unless LASTBLK or LASTLW happens to be 0, in
which case the relevant loop doesn't execute at all. Re-check your
loop conditions.


Sometimes things are _that_ simple.
Fat fingers forgot about typing the '<'! Doh!

.... i <= LASTBLK ...
.... j <= LASTLW ...

Apologies.
Yes, that's precisely what you should be doing. There's usually a
small memory overhead, and a large time overhead, associated with
every call to malloc().

Hmm. Good point, thanks again.

-Andreas
 
P

Peter Nilsson

[email protected] (Richard Harter) said:
Unfortunately, no.  The rule is very simple; you need
one call to free for each call to malloc.

It's unfortunate (for me) that C99 changed the semantics
of realloc in the case where the size argument is zero.
I have (C99 incompatible) code that used realloc to do
both the allocation and deallocation. Of course, you
still had to call the function to set the size of the
object to 'zero' to perform the deallocation.
 
A

Andreas Eibach

Eric Sosman said:
To cut a long story short: I simply forgot to post that.
No malloc()'ing at this point.

[*] entityStru_t e;
entityStru_t * pE = &e;

pE now points to the structure, and AFAIK not needs any malloc'ing here,
because line [*] has reserved mem through 'e' for the (naked) structure.

Thanks for the clarification.

You're welcome.
Now I can answer definitively
and without any guesswork whatsoever: `free(pE)' is wrong. Utterly,
hopelessly wrong. So wrong it wraps the wrongmeter's wrongneedle
around the wrongpeg and sets the little wrongbell jingling a cheery
"rongrongrong."

Hey don't make fun of Thai, will ya! :p
("rong ...." could actually mean something in their langauge, I wouldn't be
surprised ;))

Seriously, you guys rule. Well, the "tone winds" are sometimes blowing bit
strong over here, but one reaps what he sows, doesn't he? I guess I'm gonna
stay on here a little bit longer .. would not surprise me if I need you guys
again sometime. Documentation is great, but usually it won't go beyond level
1; that is, you learn about structures, and all their bells and whistles,
but not about structures IN structures.
Questions like mine usually would come up if I try to combine things in a
less-or-more advanced way.

-Andreas
 
L

lawrence.jones

Peter Nilsson said:
It's unfortunate (for me) that C99 changed the semantics
of realloc in the case where the size argument is zero.

How do you think the semantics changed? The description was changed to
hopefully make them clearer, but there wasn't any deliberate change to
them that I remember.
 
P

Peter Nilsson

How do you think the semantics changed?

For non null pointers: in C90, realloc(..., 0) freed the
block and returned a null pointer; in C99, it does a
malloc(0). It remains implementation defined whether
malloc(0) behaves as if a nonzero size was supplied.
 
N

Nick Keighley

     for (i 0; i = LASTBLK; i++)
     {  ...
          for (j= 0; j = LASTLW; j++)
         {   ...
                 pE->block->lword[j] = malloc (sizeof(unsigned
long));
I presume this is not your actual code,
You're right about that it's shortened a lot.

It's not just shortened, it's wrong. As written, both the inner and
outer loops are infinite, unless LASTBLK or LASTLW happens to be 0, in
which case the relevant loop doesn't execute at all.  Re-check your
loop conditions.

Sometimes things are _that_ simple.
Fat fingers forgot about typing the '<'! Doh!

... i <= LASTBLK ...
... j <= LASTLW ...

Apologies.


YM
for (i = 0; i < LASTBLK; i++)

ie. no '=' needed

<snip>
 
H

Hallvard B Furuseth

Peter Nilsson said:
For non null pointers: in C90, realloc(..., 0) freed the
block and returned a null pointer;
Yes...

in C99, it does a malloc(0).

No. It is realloc(NULL, size) which behaves like malloc(size).

The only time C99 7.20.3.4 says realloc() can _free_ the old object is
when it returns a new object. NULL return is failure return only.
So it doesn't matter to realloc that C99 7.20.3 (Memory management
functions) says size=0 can give either NULL or a memory object.

Otherwise, realloc(ptr, 0) return NULL either after freeing and not
freeing the object - so the program couldn't tell if it should free the
object. But then it likely can't be sure anyway, since even if the
compiler says it's a C99 compiler, it may still be using a C89 library.

So, just don't use realloc(ptr, 0) anymore.
It remains implementation defined whether
malloc(0) behaves as if a nonzero size was supplied.

True but irrelevant to C99 realloc.
 
P

Peter Nilsson

Hallvard B Furuseth said:
No.  It is realloc(NULL, size) which behaves like
malloc(size).

The only time C99 7.20.3.4 says realloc() can _free_
the old object is when it returns a new object.

AFAICS, 7.20.3p1 applies.
 NULL return is failure return only.

Why do you say that?

Note the C89 draft is similarly worded to C99 with
regards to the return value from realloc. The latter
is just more clear in saying the 'same' pointer may
be returned.
So it doesn't matter to realloc that C99 7.20.3
(Memory management functions) says size=0 can give
either NULL or a memory object.

I can't see how you can dismiss it.
Otherwise, realloc(ptr, 0) return NULL either after
freeing and not freeing the object - so the program
couldn't tell if it should free the object.

You've lost me. "If memory for the new object cannot
be allocated, the old object is not deallocated..."

So realloc(ptr, 0) does a malloc(0). If that fails, ptr
is returned; if it succeeds the old object is deallocated
and a new pointer is returned.

So, just don't use realloc(ptr, 0) anymore.

I don't. But for different reasoning it seems. :)
 
H

Hallvard B Furuseth

Peter said:
AFAICS, 7.20.3p1 applies.


Why do you say that?

Note the C89 draft is similarly worded to C99 with
regards to the return value from realloc.

That is not the point. The point is: Look for when the standards say
realloc() may free the old object. realloc() can't free an object just
because it looks like a good idea, unless the standard says it can.

C89 explicitly says: "If size is zero and ptr is not a null pointer, the
object it points to is freed."

C99 says the old object is freed only when a new object is returned:
"The realloc function deallocates the old object pointed to by ptr and
returns a pointer to a new object that has the size specified by size.
If memory for the new object cannot be allocated, the old object is
not deallocated and its value is unchanged."

Therefore if C99 realloc(non-null, 0) returns NULL, this is a failure
return. OTOH if C99 realloc(NULL, 0) returns NULL, that can be
non-failure equivalent to malloc(0).
The latter is just more clear in saying the 'same' pointer may be
returned.

Except that NULL does not point to an object.
(..snip..)

You've lost me. "If memory for the new object cannot
be allocated, the old object is not deallocated..."

So realloc(ptr, 0) does a malloc(0).

No, because:
If that fails, ptr is returned;

No, if that fails realloc returns NULL and the object is unchanged.
if it succeeds the old object is deallocated
and a new pointer is returned.

If it succeeds the old object is deallocated, and it is
implementation-defined whether malloc(0) returns an object or NULL.
 
H

Hallvard B Furuseth

I said:
If it succeeds the old object is deallocated, and it is
implementation-defined whether malloc(0) returns an object or NULL.

I should have said, that is how it would be if it was defined to do
malloc(0).
 
H

Harald van Dijk

AFAICS, 7.20.3p1 applies.

I agree with Hallvard's reply to this, but would like to try expressing
this in a slightly different way: 7.20.3p1 does apply. It gives
implementations permission to unconditionally fail to realloc to 0 bytes.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top