PATCH: Memory leak appending original mail

PATCH: Memory leak appending original mail

[Ben Schmidt]

[-- Attachment #1: Type: text/plain, Size: 45 bytes --]

I found a memory leak. Patch attached.
Ben.


[-- Attachment #2: mlmmj-origmail-memleak.patch --]
[-- Type: text/x-patch, Size: 298 bytes --]

diff -r 99ec9f826d35 src/prepstdreply.c
--- a/src/prepstdreply.c	Tue Jan 26 18:25:03 2010 +1100
+++ b/src/prepstdreply.c	Tue Jan 26 20:54:10 2010 +1100
@@ -145,6 +145,7 @@
 				value = concatstr(3, value, " ", str);
 				if(tmp)
 					myfree(tmp);
+				myfree(str);
 				count++;
 			}
 			if(str)

Re: PATCH: Memory leak appending original mail

[Ben Schmidt]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

How tremendously embarrassing...

Note to self: The correct way to fix a memory leak is not to introduce a double free.

Revised patch attached.

Ben.



On 26/01/10 9:02 PM, Ben Schmidt wrote:
> I found a memory leak. Patch attached.
> Ben.
>

[-- Attachment #2: mlmmj-origmail-memleak.patch --]
[-- Type: text/x-patch, Size: 315 bytes --]

diff -r f48f18a405ee src/prepstdreply.c
--- a/src/prepstdreply.c	Wed Feb 03 14:57:37 2010 +1100
+++ b/src/prepstdreply.c	Wed Feb 03 14:59:45 2010 +1100
@@ -145,6 +145,8 @@
 				value = concatstr(3, value, " ", str);
 				if(tmp)
 					myfree(tmp);
+				myfree(str);
+				str = NULL;
 				count++;
 			}
 			if(str)

Re: PATCH: Memory leak appending original mail

[Morten Shearman Kirkegaard]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Wed, 2010-02-03 at 15:02 +1100, Ben Schmidt wrote:
> How tremendously embarrassing...
> 
> Note to self: The correct way to fix a memory leak is not to introduce
> a double free.

True :-)

> Revised patch attached.

I looked at the code, and got thinking. Since the loop is only entered
if str is not NULL, and str is only used inside the loop, couldn't we
just free str inside the loop?

Proposed patch attached.

Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/

[-- Attachment #2: patch-mlmmj-1.2.17-origmail.diff --]
[-- Type: text/x-patch, Size: 591 bytes --]

Index: src/prepstdreply.c
===================================================================
RCS file: /home/mmj/MMJCVS/mlmmj/src/prepstdreply.c,v
retrieving revision 1.28
diff -u -r1.28 prepstdreply.c
--- src/prepstdreply.c	5 Nov 2007 21:23:37 -0000	1.28
+++ src/prepstdreply.c	16 Feb 2010 06:13:27 -0000
@@ -145,10 +145,9 @@
 				value = concatstr(3, value, " ", str);
 				if(tmp)
 					myfree(tmp);
+				myfree(str);
 				count++;
 			}
-			if(str)
-				myfree(str);
 			close(mailfd);
 		}else{
 			log_error(LOG_ARGS, "Could not substitute $originalmail$ (mailname == %s)",mailname);

Re: PATCH: Memory leak appending original mail

[Ben Schmidt]

> I looked at the code, and got thinking. Since the loop is only entered
> if str is not NULL, and str is only used inside the loop, couldn't we
> just free str inside the loop?
>
> Proposed patch attached.

Yeah, that works! Go for it!

Ben.

Re: PATCH: Memory leak appending original mail

[Morten Shearman Kirkegaard]

On Tue, 2010-02-16 at 18:35 +1100, Ben Schmidt wrote:
> > I looked at the code, and got thinking. Since the loop is only entered
> > if str is not NULL, and str is only used inside the loop, couldn't we
> > just free str inside the loop?
> >
> > Proposed patch attached.
> 
> Yeah, that works! Go for it!

Committed to CVS.


Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/