Re: Memory Leak in FreeAltScreen

Bill Pursell <[email protected]> Mon, 27 Mar 2006 17:03:33 +0100
Newsgroups gmane.comp.gnu.screen
Message-ID <[email protected]>
Juergen Weigert wrote:
> On Mar 27, 06 05:41:04 +0100, Bill Pursell wrote:
> 
>>Daniel De Graaf wrote:
>>
>>>In FreeAltScreen, the w_alt_mlines and w_alt_hlines entries are not
>>>freed before setting them to zero. This causes a memory leak every
>>>time a screen is switched, which can be demonstrated by the shell
>>>script "for a in `seq 1 1000`; do echo test|less -E;done".
>>>
>>>- Daniel De Graaf
>>>
>>>--- resize.c.orig	2003-09-08 09:26:31.000000000 -0500
>>>+++ resize.c	2006-03-26 13:16:26.000000000 -0600
>>>@@ -1045,8 +1045,10 @@
>>>
>>>  if (p->w_alt_mlines)
>>>    for (i = 0; i < p->w_alt_height; i++)
>>>      FreeMline(p->w_alt_mlines + i);
>>>+  free(p->w_alt_mlines);
>>>+  free(p->w_alt_hlines);
>>>  p->w_alt_mlines = 0;
>>>  p->w_alt_width = 0;
>>>  p->w_alt_height = 0;
>>>  p->w_alt_x = 0;
>>>
>>>
>>
>>Freeing a memory address and then assigning to it is a bad idea.  Simply 
>>freeing it is adequate.
> 
> 
> why bad idea? I prefer to die in the NULL-Pointer access, 
> than go amok in memory that I do no longer own.
> 
>         cheers,
>

My bad.  I mis-read the code as:

free(p->w_alt_mlines);
*p->w_alt_mlines = 0;

Which is definitely a bad idea :)