[tpop3d-discuss] memory leak fixing in 1.5.1

Chris Lightfoot chris at ex-parrot.com
Fri, 29 Aug 2003 09:11:00 +0100


On Wed, Aug 27, 2003 at 02:44:54PM -0400, Kevin Bonner wrote:
> 
> Testing with valgrind, I saw numerous memory leaks that needed fixing.  I had 
> two tests, one where I just start the server and kill it, and one where I 
> start the server, do a successful connection, and disconnect.
> 
> Attached is a patch to resolve several of the memory leaks I saw.  It is a 
> patch for the 1.5.1 source.  I see that the CVS head has the SSL_CTX_free 
> call, but as ctx isn't referenced in the tls.c file, it probably won't be 
> able to find that reference.

Thank you for these. Note a couple of your patches are not
correct:

> diff -urN tpop3d-1.5.1/auth_perl.c tpop3d-1.5.1.new/auth_perl.c
> --- tpop3d-1.5.1/auth_perl.c	Thu Jan  9 17:59:37 2003
> +++ tpop3d-1.5.1.new/auth_perl.c	Wed Aug 27 13:32:33 2003
> @@ -98,7 +98,7 @@
>      }
>  
>      /* Put a useful string into the environment. */
> -    putenv(xstrdup("TPOP3D_CONTEXT=auth_perl"));
> +    setenv("TPOP3D_CONTEXT","auth_perl",1);

There was a reason not to use setenv here, though I can't
remember what it was (portability?). In any case this is
only called once and the loss of 24 bytes of memory is not
a serious problem.

> diff -urN tpop3d-1.5.1/connection.c tpop3d-1.5.1.new/connection.c
> --- tpop3d-1.5.1/connection.c	Mon Jul 14 19:31:20 2003
> +++ tpop3d-1.5.1.new/connection.c	Wed Aug 27 13:32:39 2003
> @@ -319,6 +319,8 @@
>  
>      pc = pop3command_new(p);
>  
> +    xfree(line);

(In connection_parsecommand().) No. line is a static
pointer; it is passed to buffer_consume_to_mark, which
may reallocate it, but if it is large enough, will re-use
the storage. Freeing it here will cause problems because
buffer_consume_to_mark will either write into the free'd
memory, or pass its address to realloc; both possibilities
are undefined behaviour.

The idea of this code is that the buffer pointed to by
line grows as large as is needed to receive lines from the
client, but no larger; at this point, we never spend time
in malloc/free.

-- 
Something Went Wrong in Jet Crash, Expert Says (newspaper headline)