PATCH: Memorey leak with PS4 usage.

PATCH: Memorey leak with PS4 usage.

[Felix Rosencrantz]

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

There seems to be a memory leak issue with the use of promptexpand.
I've included a patch for utils.c.

I think there are additional problems with promptexpand's use in subst.c, which
I'll leave for someone who understands the code better.

-FR


__________________________________________________
Do You Yahoo!?
Send instant messages & get email alerts with Yahoo! Messenger.
http://im.yahoo.com/

[-- Attachment #2: patch5.txt --]
[-- Type: text/plain, Size: 271 bytes --]

Index: Src/utils.c
===================================================================
--- zsh/Src/outils.c	Sat May 20 08:56:03 2000
+++ zsh/Src/utils.c	Wed May 31 20:12:20 2000
@@ -807,6 +807,7 @@
 	opts[XTRACE] = t;
 
 	fprintf(xtrerr, "%s", s);
+	free(s);
     }
 }
 

Re: PATCH: Memorey leak with PS4 usage.

[Bart Schaefer]

On May 31,  8:27pm, Felix Rosencrantz wrote:
} Subject: PATCH: Memorey leak with PS4 usage.
}
} There seems to be a memory leak issue with the use of promptexpand.
} I've included a patch for utils.c.

What leads you to this conclusion?

dupstring() allocates off zsh's internal heap.  Freeing a pointer off the
heap is a Bad Thing.  The heap is allocated and freed in large chunks that
zsh subdivides as necessary.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: Memorey leak with PS4 usage.

[Peter Stephenson]

> > } There seems to be a memory leak issue with the use of promptexpand.
> > } I've included a patch for utils.c.
> I think the these lines in promptexpand:
> Are not allocated off the internal heap.

Yes, this needs freeing, as do the other uses in subst.c below.
promptexpand always allocates its own storage, and unmetafy returns the
same area of memory.

I think I'll commit these, too, on the basis that if this is wrong then I
ought to go and do something else like flower-arranging.

Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.7
diff -u -r1.7 subst.c
--- Src/subst.c	2000/05/31 08:56:24	1.7
+++ Src/subst.c	2000/06/01 09:01:20
@@ -1661,18 +1661,24 @@
 		aval = arrdup(aval), copied = 1;
 	    ap = aval;
 	    for (; *ap; ap++) {
+		char *tmps;
 		unmetafy(*ap, &len);
 		untokenize(*ap);
-		*ap = unmetafy(promptexpand(metafy(*ap, len, META_NOALLOC),
-					    0, NULL, NULL), &len);
+		tmps = unmetafy(promptexpand(metafy(*ap, len, META_NOALLOC),
+					     0, NULL, NULL), &len);
+		*ap = dupstring(tmps);
+		free(tmps);
 	    }
 	} else {
+	    char *tmps;
 	    if (!copied)
 		val = dupstring(val), copied = 1;
 	    unmetafy(val, &len);
 	    untokenize(val);
-	    val = unmetafy(promptexpand(metafy(val, len, META_NOALLOC),
+	    tmps = unmetafy(promptexpand(metafy(val, len, META_NOALLOC),
 					0, NULL, NULL), &len);
+	    val = dupstring(tmps);
+	    free(tmps);
 	}
 	opts[PROMPTSUBST] = ops;
 	opts[PROMPTBANG] = opb;

-- 
Peter Stephenson <pws@cambridgesiliconradio.com>
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070

Re: PATCH: Memorey leak with PS4 usage.

[Felix Rosencrantz]

--- Bart Schaefer <schaefer@candle.brasslantern.com> wrote:
> On May 31,  8:27pm, Felix Rosencrantz wrote:
> } Subject: PATCH: Memorey leak with PS4 usage.
> }
> } There seems to be a memory leak issue with the use of promptexpand.
> } I've included a patch for utils.c.
> 
> What leads you to this conclusion?
> 
> dupstring() allocates off zsh's internal heap.  Freeing a pointer off the
> heap is a Bad Thing.  The heap is allocated and freed in large chunks that
> zsh subdivides as necessary.

I think the these lines in promptexpand:
prompt.c:144:        return ztrdup("");
prompt.c:163:   bp =bufline = buf = zcalloc(bufspc = 256);

Are not allocated off the internal heap.

Also, other uses of promptexpand use free to free this space. For example,
in bin_print():
builtin.c:2758   if(ops['P']) {
builtin.c:2759       /*
builtin.c:2760        * promptexpand uses permanent storage: to avoid
builtin.c:2761        * messy memory management, stick it on the heap
builtin.c:2762        * instead.
builtin.c:2763        */
builtin.c:2764        char *str = unmetafy(promptexpand(metafy(args[n], len[n],
builtin.c:2765                               META_NOALLOC), 0, NULL, NULL),
&len[n]);
builtin.c:2766        args[n] = dupstring(str);
builtin.c:2767        free(str);
builtin.c:2768   }

Is this correct? Plus, that comment is unclear to me.  Though code speaks
louder than comments. 

-FR.


__________________________________________________
Do You Yahoo!?
Send instant messages & get email alerts with Yahoo! Messenger.
http://im.yahoo.com/