* Question: patch for memory leak @ 1999-01-15 2:17 Daniel X. Pape 1999-01-15 15:20 ` PATCH: was " Daniel X. Pape 0 siblings, 1 reply; 4+ messages in thread From: Daniel X. Pape @ 1999-01-15 2:17 UTC (permalink / raw) To: zsh-workers Hi - I recently downloaded and compiled zsh-3.1.5-pws-4. However, I found a couple of memory leaks in builtin.c where the result of a ztrdup was being used as an argument, and not freed, and also where the result of a "promptexpand" was being passed to "unmetafy," and then that result was never freed. After fixing the problem and re-compiling, I have been using the resulting executable for two days now, and have seen no problems. Is there any kind of test suite to use before submitting a patch? or should I just go ahead and send it? Thanks - Dan ^ permalink raw reply [flat|nested] 4+ messages in thread
* PATCH: was Re: Question: patch for memory leak 1999-01-15 2:17 Question: patch for memory leak Daniel X. Pape @ 1999-01-15 15:20 ` Daniel X. Pape 1999-01-15 15:44 ` PATCH: zsh-3.1.5-pws-4 + typeset fix: " Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Daniel X. Pape @ 1999-01-15 15:20 UTC (permalink / raw) To: zsh-workers On Thu, 14 Jan 1999, Daniel X. Pape wrote: > I recently downloaded and compiled zsh-3.1.5-pws-4. However, I found a > couple of memory leaks in builtin.c where the result of a ztrdup was > being used as an argument, and not freed, and also where the result of a > "promptexpand" was being passed to "unmetafy," and then that result was > never freed. > > After fixing the problem and re-compiling, I have been using the > resulting executable for two days now, and have seen no problems. > > Is there any kind of test suite to use before submitting a patch? or > should I just go ahead and send it? Here is the patch - let me know if it helps: *** builtin.orig.c Thu Jan 14 20:04:36 1999 --- builtin.c Thu Jan 14 19:52:35 1999 *************** *** 1646,1651 **** --- 1646,1652 ---- setsparam(asg->name, ztrdup(asg->value)); } } else { + char *t; if (bit) { if (pm->flags & PM_READONLY) { on |= ~off & PM_READONLY; *************** *** 1657,1663 **** } /* create a new node for a parameter with the * * flags in `on' minus the readonly flag */ ! pm = createparam(ztrdup(asg->name), on & ~PM_READONLY); DPUTS(!pm, "BUG: parameter not created"); pm->ct = auxlen; if (func != BIN_EXPORT) --- 1658,1666 ---- } /* create a new node for a parameter with the * * flags in `on' minus the readonly flag */ ! t = ztrdup(asg->name); ! pm = createparam(t, on & ~PM_READONLY); ! zsfree(t); DPUTS(!pm, "BUG: parameter not created"); pm->ct = auxlen; if (func != BIN_EXPORT) *************** *** 2300,2305 **** --- 2303,2309 ---- int *len; Histent ent; FILE *fout = stdout; + char *arg_to_free = 0; /* -m option -- treat the first argument as a pattern and remove * arguments not matching */ *************** *** 2328,2336 **** args[n] = getkeystring(args[n], &len[n], func != BIN_ECHO && !ops['e'], &nnl); /* -P option -- interpret as a prompt sequence */ ! if(ops['P']) ! args[n] = unmetafy(promptexpand(metafy(args[n], len[n], ! META_NOALLOC), 0, NULL, NULL), &len[n]); /* -D option -- interpret as a directory, and use ~ */ if(ops['D']) { Nameddir d = finddir(args[n]); --- 2332,2342 ---- args[n] = getkeystring(args[n], &len[n], func != BIN_ECHO && !ops['e'], &nnl); /* -P option -- interpret as a prompt sequence */ ! if(ops['P']) { ! arg_to_free = unmetafy(promptexpand(metafy(args[n], len[n], ! META_NOALLOC), 0, NULL, NULL), &len[n]); ! args[n] = arg_to_free; ! } /* -D option -- interpret as a directory, and use ~ */ if(ops['D']) { Nameddir d = finddir(args[n]); *************** *** 2349,2354 **** --- 2355,2362 ---- PERMALLOC { pushnode(bufstack, sepjoin(args, NULL)); } LASTALLOC; + if(arg_to_free) + free(arg_to_free); return 0; } /* -s option -- add the arguments to the history list */ *************** *** 2378,2383 **** --- 2386,2393 ---- ent->stim = ent->ftim = time(NULL); ent->flags = 0; } LASTALLOC; + if(arg_to_free) + free(arg_to_free); return 0; } /* -u and -p -- output to other than standard output */ *************** *** 2392,2401 **** --- 2402,2415 ---- fd = coprocout; if ((fd = dup(fd)) < 0) { zwarnnam(name, "bad file number", NULL, 0); + if(arg_to_free) + free(arg_to_free); return 1; } if ((fout = fdopen(fd, "w")) == 0) { zwarnnam(name, "bad mode on fd", NULL, 0); + if(arg_to_free) + free(arg_to_free); return 1; } } *************** *** 2448,2453 **** --- 2462,2469 ---- } if (fout != stdout) fclose(fout); + if(arg_to_free) + free(arg_to_free); return 0; } /* normal output */ *************** *** 2460,2465 **** --- 2476,2483 ---- fputc(ops['N'] ? '\0' : '\n', fout); if (fout != stdout) fclose(fout); + if(arg_to_free) + free(arg_to_free); return 0; } ^ permalink raw reply [flat|nested] 4+ messages in thread
* PATCH: zsh-3.1.5-pws-4 + typeset fix: memory leak 1999-01-15 15:20 ` PATCH: was " Daniel X. Pape @ 1999-01-15 15:44 ` Peter Stephenson 1999-01-15 16:43 ` Bart Schaefer 0 siblings, 1 reply; 4+ messages in thread From: Peter Stephenson @ 1999-01-15 15:44 UTC (permalink / raw) To: Daniel X. Pape, zsh-workers "Daniel X. Pape" wrote: > On Thu, 14 Jan 1999, Daniel X. Pape wrote: > > > I recently downloaded and compiled zsh-3.1.5-pws-4. However, I found a > > couple of memory leaks in builtin.c where the result of a ztrdup was > > being used as an argument, and not freed, and also where the result of a > > "promptexpand" was being passed to "unmetafy," and then that result was > > never freed. > > Here is the patch - let me know if it helps: You're certainly right about the memory leaks --- thanks. I've rewritten the patch a bit: the first part now fits in with a recent patch of mine in zsh-workers/4902, and also I don't think you actually need to duplicate the parameter name at all (somebody must simply have forgotten that createparam() does that inside). In the second case, the problem with your approach is that `print -P' can take more than one argument, and all need freeing: the easiest thing to do is use heap memory (which gets freed automatically when the builtin exits), and free the permanent memory straight away. *** Src/builtin.c.mem Thu Jan 14 15:55:34 1999 --- Src/builtin.c Fri Jan 15 16:29:33 1999 *************** *** 1560,1566 **** * Create a new node for a parameter with the flags in `on' minus the * readonly flag */ ! pm = createparam(ztrdup(pname), on & ~PM_READONLY); DPUTS(!pm, "BUG: parameter not created"); pm->ct = auxlen; if (func != BIN_EXPORT) --- 1560,1566 ---- * Create a new node for a parameter with the flags in `on' minus the * readonly flag */ ! pm = createparam(pname, on & ~PM_READONLY); DPUTS(!pm, "BUG: parameter not created"); pm->ct = auxlen; if (func != BIN_EXPORT) *************** *** 2346,2354 **** args[n] = getkeystring(args[n], &len[n], func != BIN_ECHO && !ops['e'], &nnl); /* -P option -- interpret as a prompt sequence */ ! if(ops['P']) ! args[n] = unmetafy(promptexpand(metafy(args[n], len[n], ! META_NOALLOC), 0, NULL, NULL), &len[n]); /* -D option -- interpret as a directory, and use ~ */ if(ops['D']) { Nameddir d = finddir(args[n]); --- 2346,2362 ---- args[n] = getkeystring(args[n], &len[n], func != BIN_ECHO && !ops['e'], &nnl); /* -P option -- interpret as a prompt sequence */ ! if(ops['P']) { ! /* ! * promptexpand uses permanent storage: to avoid ! * messy memory management, stick it on the heap ! * instead. ! */ ! char *str = unmetafy(promptexpand(metafy(args[n], len[n], ! META_NOALLOC), 0, NULL, NULL), &len[n]); ! args[n] = dupstring(str); ! free(str); ! } /* -D option -- interpret as a directory, and use ~ */ if(ops['D']) { Nameddir d = finddir(args[n]); -- Peter Stephenson <pws@ibmth.df.unipi.it> Tel: +39 050 844536 WWW: http://www.ifh.de/~pws/ Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: zsh-3.1.5-pws-4 + typeset fix: memory leak 1999-01-15 15:44 ` PATCH: zsh-3.1.5-pws-4 + typeset fix: " Peter Stephenson @ 1999-01-15 16:43 ` Bart Schaefer 0 siblings, 0 replies; 4+ messages in thread From: Bart Schaefer @ 1999-01-15 16:43 UTC (permalink / raw) To: Peter Stephenson, zsh-workers On Jan 15, 4:44pm, Peter Stephenson wrote: } Subject: PATCH: zsh-3.1.5-pws-4 + typeset fix: memory leak } } the problem with your approach is that `print -P' can take more than } one argument, and all need freeing: the easiest thing to do is use } heap memory (which gets freed automatically when the builtin exits), } and free the permanent memory straight away. Another place where 3.0.5 appears to have been patched but 3.1.5 has not. The 3.0.5 code reads: /* -P option -- interpret as a prompt sequence */ if(ops['P']) { char *arg = putprompt(metafy(args[n], len[n], META_NOALLOC), &len[n], NULL, 0); args[n] = (char *)alloc(len[n] + 1); memcpy(args[n], arg, len[n]); args[n][len[n]] = 0; free(arg); } I'm sure the memcpy() is done to avoid recomputing len[n]+1 in dupstring(). -- Bart Schaefer Brass Lantern Enterprises http://www.well.com/user/barts http://www.brasslantern.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~1999-01-15 16:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 1999-01-15 2:17 Question: patch for memory leak Daniel X. Pape 1999-01-15 15:20 ` PATCH: was " Daniel X. Pape 1999-01-15 15:44 ` PATCH: zsh-3.1.5-pws-4 + typeset fix: " Peter Stephenson 1999-01-15 16:43 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).