«

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[i]->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

"Andreas Eibach" <> writes:

> 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[i]->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.

Andreas Eibach wrote:
> 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[i]->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[i]->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.

"Malcolm McLean" <> wrote:
> 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[i]->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

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.

"Nate Eldredge" <> schrieb im Newsbeitrag
news:...
> "Andreas Eibach" <> writes:
>
> > 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[i]->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

Andreas Eibach wrote:
> "Nate Eldredge" <> schrieb im Newsbeitrag
> news:...
> > "Andreas Eibach" <> writes:
....|
> > > 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[i]->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.

....
> > 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.

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().

"Andreas Eibach" <> writes:

> "Nate Eldredge" <> schrieb im Newsbeitrag
> news:...
>> "Andreas Eibach" <> writes:
>> > for (i 0; i = LASTBLK; i++)
>> > { ...
>> > for (j= 0; j = LASTLW; j++)
>> > { ...
>> > pE->block[i]->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

"Eric Sosman" <> wrote:
> 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[i]->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[i]'.
OK, granted.
Gladly I have not forgotten about it, but I've just realized that I do the
block[i] and block[i]->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.

-Andreas

"jameskuyper" <> wrote:
> 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[i]->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.

> > 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().

Hmm. Good point, thanks again.

-Andreas