On Sat, 28 Sep 2002, Vertigo wrote:
> I've been looking at a piece of code for a couple of days now that
> appears to be
> leaking memory. I'd just like another opinion it to see what it is I'm
> not
> seeing. It's tied into my do_look routine and splices two strings
> together
> using getline OLC 1.81's standard getline and other standard functions.
> I'm
> just thinking something isn't being freed correctly. What is happening
> is
> I'm getting an increase in perms by two every single time the character
> 'looks'. Which is equivalent to the amount of strings I'm sending to
> this
> piece of code. The other two strings display fine with no leak on their
> own
> so I've pretty much deduced that this one is the culprit.
>
> Vert
>
>
> Attached below:
> [Snip]
There are two things you need to learn about allocating and freeing memory.
The first thing is that everything you allocate must be freed exactly
once. The second thing is that you can't free something you didn't
allocate.
Let's just step through this function looking at allocated memory.
Before the function starts, nothing is allocated (we assume the
calling function is responsible for the variables it passes in).
memory allocated and not yet freed
----------------------------------
none
> char *insert_string( const char *oldstring, char *insert )
> {
> char formatted[MSL];
> char *tempstr, *therest;
> char templine[MSL], newstr[MSL];
> char temp1[MSL], temp2[MSL];
> bool mapbigger = TRUE;
> bool cap = FALSE;
> int line= 0, i=0;
>
> tempstr = str_dup(oldstring);
str_dup() allocates memory that must be freed with free_string(),
so we need to keep track of it.
memory allocated and not yet freed
----------------------------------
tempstr
> tempstr = format_string2( tempstr, 72, FALSE );
> /* format it to minus the map */
format_string2() frees the string passed to it, then allocates
a new one. So the memory tempstr was allocated a few lines ago
is now freed, and new memory is allocated for it.
memory allocated and not yet freed
----------------------------------
tempstr
> newstr[0] = '\0';
>
> while( *insert || *tempstr )
> {
> if( *tempstr && !(*insert) )
> mapbigger = FALSE;
>
> insert = getline(insert, temp1); /*gets first line of map */
>
> tempstr = getline(tempstr, temp2); /*get first line of desc*/
Since you just changed tempstr, which was the only pointer to the
memory allocated about a dozen lines ago, from now on I will refer
to that address as "the address formerly known as tempstr".
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
> sprintf(templine, "%s %s{x\n\r", temp1, temp2);
>
> strcat(newstr, templine);
> }
>
> if(!mapbigger)
The memory allocation is different in each part of this if, so the
chart branches here. I'll show the second branch indented more.
> {
> for(tempstr = newstr;;)
> {
> formatted[i++] = *tempstr;
> if(*(tempstr++) == '\r')
> {
> if(++line > 6)
> break;
> }
> }
>
> formatted[i] = '\0';
>
> if(is_end_punct(formatted[i-3]))
> cap = TRUE;
>
> therest = str_dup(tempstr);
Here's another allocation. Let's add it to the list.
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
therest
> therest = format_string2(therest, 79, FALSE );
Once again, format_string2() frees the memory passed to it, then
allocates more memory that it returns. So the memory pointed
to by therest is freed, and therest is now pointing to another
allocated chunk of memory.
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
therest
>
> if(!cap)
> *therest = LOWER(*therest);
>
> strcat(formatted,therest);
>
> free_string(tempstr);
Hmm.. tempstr doesn't point to anything we allocated...
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
therest
memory freed that was never allocated
-------------------------------------
tempstr
> free_string(therest);
One of our allocated strings is freed here.
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
memory freed that was never allocated
-------------------------------------
tempstr
> free_string(templine);
templine doesn't point to any memory allocated here.
In fact, it points to the stack!
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
memory freed that was never allocated
-------------------------------------
tempstr
templine
> free_string(temp1);
Trying to free another variable on the stack...
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
memory freed that was never allocated
-------------------------------------
tempstr
templine
temp1
> free_string(temp2);
And another...
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
memory freed that was never allocated
-------------------------------------
tempstr
templine
temp1
temp2
> return str_dup(formatted);
Here's another allocation.
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
copy of formatted (returned)
memory freed that was never allocated
-------------------------------------
tempstr
templine
temp1
temp2
> free_string(formatted);
Since this line is never called (the function just returned!),
we'll ignore it.
> }
> else
> return str_dup(newstr);
Here's another allocation.
This is on the second branch of the if.
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
copy of newstr (returned)
> free_string(newstr);
Since this line is never called because the function has already
returned, no matter which branch of the if was taken, we'll ignore
this line.
> }
That's the end of the function.
When the function returns, it has followed one of the two branches,
and is in the following state:
Branch 1
--------
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
copy of formatted (returned)
memory freed that was never allocated
-------------------------------------
tempstr
templine
temp1
temp2
Branch 2
--------
memory allocated and not yet freed
----------------------------------
the address formerly known as tempstr
copy of newstr (returned)
As you can see, in both branches, two strings are allocated that
are never freed (as well as four addresses freed that are never
allocated in branch one!). This would account for the increase
of two strings you are seeing each time the function is called.
The address of one of the two strings allocated in either branch
is passed back to the calling function. By freeing the memory
in the calling function, one of these may be taken care of.
I would very strongly suggest that you get out your book on C
and study the chapter on pointers. Understanding the concepts
behind pointers and dynamic memory allocation will make it much
easier to figure stuff like this out.
Dennis
