🚀 go-pugleaf

RetroBBS NetNews Server

Inspired by RockSolid Light RIP Retro Guy

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?
#414
Author: Ralph Zazula
Date: Mon, 03 Aug 1992 09:02
58 lines
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?
#417
Author: Scott Hess
Date: Mon, 03 Aug 1992 07:04
163 lines
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