Thread View: comp.sys.next.programmer
2 messages
2 total messages
Started by Ralph Zazula
Mon, 03 Aug 1992 09:02
Freeing from Storage object / AcceptWindow = bad example?
Author: Ralph Zazula
Date: Mon, 03 Aug 1992 09:02
Date: Mon, 03 Aug 1992 09:02
58 lines
2324 bytes
2324 bytes
Howdy!
I'm looking for some info on freeing objects that have been placed into a
Storage object. The canonical example for this appears to be the AcceptWindow
class in the ToolInspector demo. The -free method for this class is this:
- free
{
int i;
i = [viewRectList count];
while (i--) {
free([viewRectList elementAt:i]);
}
[viewRectList free];
return [super free];
}
and the elements are added to the Storage instance (viewRectList) like so:
newViewRect = (ViewRectPair *)malloc(sizeof(ViewRectPair));
...
[viewRectList addElement:newViewRect];
where a "ViewRectPair" is:
typedef struct _ViewRectPair
{
id view;
NXRect rect;
} ViewRectPair;
Okay, so this is the background. Now the question: why does this not appear to
work? I'm seeing a few things when using this. First, MallocDebug reports
each of the malloc'ed VRP's as leaking, as in:
default 0x0079fd80 20 -[AcceptWindow registerRect:forView:]
Then, when I go through the -free method (with MallocDebug compiled in) I get a
"freed unmalloced pointer" message for each call to free() in -free. Finally,
an app using the AcceptWindow crashes in grand fashion when continuing after
freeing the the AcceptWindow. The crashes tend to occur in differing remote
parts of the app, usually during some type of memory allocation.
For now, I've just ended up freeing only the Storage instance and letting the
VRP's leak away... (i.e., removed all calls to free() in AcceptWindow's -free)
and this seems to stop the app from crashing.
So, is there a problem with AcceptWindow's -free method? Or, could there be
something else strange going on here?
Thanks,
Ralph
---
Ralph Zazula
zazula@pri.com (NeXT Mail)
Pinnacle Research, Inc.
Re: Freeing from Storage object / AcceptWindow = bad example?
Author: Scott Hess
Date: Mon, 03 Aug 1992 07:04
Date: Mon, 03 Aug 1992 07:04
163 lines
6051 bytes
6051 bytes
In article <1992Aug3.020248.3570@arizona.edu>,
zazula@pri.com (Ralph Zazula) writes:
>I'm looking for some info on freeing objects that have been placed
>into a Storage object. The canonical example for this appears to
>be the AcceptWindow class in the ToolInspector demo. The -free
>method for this class is this:
Sorry, but AcceptWindow is a _terrible_ example of usage of Storage
objects. I don't know who is responsible for putting that example in
the /NextDeveloper/Examples directory, but I sure hope they've been
talked to. It's not only badly written - it is quite simply _wrong_,
displaying an amazing lack of knowledge about how Storage works.
Don't get me wrong - I don't think anyone is somehow an idiot because
they don't understand the intricacies of Storage. Indeed, I don't
know many people who make use of Storage very effectively. I just
don't think that the _examples_ should be done by someone who doesn't
understand it ...
Here are the problems:
We start out with:
typedef struct {
id view;
NXRect rect;
} ViewRectPair;
and we allocate the Storage object with:
viewRectList = [Storage newCount:1
elementSize:sizeof(ViewRectPair)
description:"{@ffff}"];
Other than being an old-style allocation, we are fine thus far.
The only tip here is that it's often slicker to do:
viewRectList=[Storage newCount:1 elementSize:sizeof( ViewRectPair)
description:@encode( ViewRectPair)];
This way, in the future when you change ViewRectPair to add another
element or something, you don't have to change your description:
parameter.
Then, when AcceptWindow adds a rectangle/view association, it does:
- registerRect:(NXRect *)rect forView:view
{
ViewRectPair *newViewRect;
/* add the view-rect pair to our list */
newViewRect = (ViewRectPair *)malloc(sizeof(ViewRectPair));
newViewRect->view = view;
newViewRect->rect = *rect;
[viewRectList addElement:newViewRect];
return self;
}
This is _wrong_, _wrong_, _wrong_! You don't allocate extra storage
to put into the Storage object - what would be the point? -addElement:
requires a (void *) because you simply cannot pass a ViewRectPair
directly in a generic fashion. Why it leaks so badly is because
Storage is not going to keep track of your malloc'ed data - and
neither are you! The correct version is:
- registerRect:(NXRect *)rect forView:view
{
ViewRectPair newViewRect={
view, *rect
};
[viewRectList addElement:&newViewRect];
return self;
}
The Storage will handle the allocation for you, and will stuff the
data in memory that it handles.
Then, we also have:
- free
{
int i;
/* free the viewRectList */
i = [viewRectList count];
while (i--) {
free([viewRectList elementAt:i]);
}
[viewRectList free];
return [super free];
}
Again, Storage isn't returning pointers to malloc'ed data in
-elementAt:! If you use the revised -registerRect:forView: method
I give, then all you have to do is:
- free
{
[viewRectList free];
return [super free];
}
How Storage Works
Storage maintains some internal info that it's useful to understand.
elementSize and description are obvious - those are data you passed
into the +new* or -init* method. dataPtr is a pointer to malloc'ed
memory. The size of the memory is elementSize*maxElements bytes.
Of that, elementSize*numElements bytes are currently in use.
So, how's an -addElement: work? First, Storage will check to see
if there's already room - if maxElements>numElements, then we don't
have to allocate more data. If there isn't room, then maxElements
is increased, and dataPtr is realloc'ed to the new size. Once
there's certain to be enough room, it copies the data at the passed
pointer into the new slot! It does _not_ pay any attention to the
pointer itself. Scott's Own Rendition of -addElement:
- addElement:(void *)anElement
{
/* If no more room, add some room. Watch out for the case
* when we have _no_ room - 0*2==0, so I just stuff in a
* sensible alternative (8).
*/
if( numElements==maxElements) {
[self setAvailableCapacity:maxElements ? maxElements*2 : 8];
}
/* Store away the passed element. */
memcpy( ((unsigned char *)dataPtr)+elementSize*numElements, anElement, elementSize);
/* There's now one more element. */
numElements++;
return self;
}
Now, -elementAt: is just returning a reference into the array.
Something like:
-(void *)elementAt:(unsigned)index
{
if( index<numElements) {
return ((unsigned char *)dataPtr)+elementSize*index;
}
return NULL;
}
A natural fallout of this is that [aStorage elementAt:0]==aStorage->dataPtr.
Others, like -replace:at:, are quite simple, also. In -replace:at:,
you just copy right over top of the old element. -insert:at: would
add more capacity as needed, then memcpy() the current elements
out one slot, and put the new element there in the opened gap.
-removeAt: does the inverse of -insert:at:, copying data over top
of the removed element.
Later,
--
scott hess <shess@ssesco.com> <Who's now in a programming Berserker-frenzy>
12901 Upton Avenue South, #326 Burnsville, MN 55337 (612) 895-1208 Anytime!
<I want to become so famous that people buy tapes of me reading source code>
Thread Navigation
This is a paginated view of messages in the thread with full content displayed inline.
Messages are displayed in chronological order, with the original post highlighted in green.
Use pagination controls to navigate through all messages in large threads.
Back to All Threads