From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4041 invoked from network); 14 Apr 2008 13:34:58 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.4 (2008-01-01) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.4 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 14 Apr 2008 13:34:58 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 40818 invoked from network); 14 Apr 2008 13:34:52 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 14 Apr 2008 13:34:52 -0000 Received: (qmail 25816 invoked by alias); 14 Apr 2008 13:34:49 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 24814 Received: (qmail 25800 invoked from network); 14 Apr 2008 13:34:48 -0000 Received: from bifrost.dotsrc.org (130.225.254.106) by sunsite.dk with SMTP; 14 Apr 2008 13:34:48 -0000 Received: from cluster-g.mailcontrol.com (cluster-g.mailcontrol.com [85.115.41.190]) by bifrost.dotsrc.org (Postfix) with ESMTP id 54BEA8043AC7 for ; Mon, 14 Apr 2008 15:34:45 +0200 (CEST) Received: from cameurexb01.EUROPE.ROOT.PRI ([62.189.241.200]) by rly20g.srv.mailcontrol.com (MailControl) with ESMTP id m3EDYJl5015418 for ; Mon, 14 Apr 2008 14:34:31 +0100 Received: from news01.csr.com ([10.103.143.38]) by cameurexb01.EUROPE.ROOT.PRI with Microsoft SMTPSVC(6.0.3790.3959); Mon, 14 Apr 2008 14:34:26 +0100 Received: from news01.csr.com (localhost.localdomain [127.0.0.1]) by news01.csr.com (8.14.2/8.13.4) with ESMTP id m3EDYRSI006756 for ; Mon, 14 Apr 2008 14:34:27 +0100 Received: from csr.com (pws@localhost) by news01.csr.com (8.14.2/8.14.2/Submit) with ESMTP id m3EDYRJj006753 for ; Mon, 14 Apr 2008 14:34:27 +0100 Message-Id: <200804141334.m3EDYRJj006753@news01.csr.com> X-Authentication-Warning: news01.csr.com: pws owned process doing -bs To: zsh-workers@sunsite.dk Subject: Re: Possible memory leak in hist.c In-reply-to: <20080414132735.GT1223@prunille.vinc17.org> References: <20080414132735.GT1223@prunille.vinc17.org> Comments: In-reply-to Vincent Lefevre message dated "Mon, 14 Apr 2008 15:27:35 +0200." Date: Mon, 14 Apr 2008 14:34:27 +0100 From: Peter Stephenson X-OriginalArrivalTime: 14 Apr 2008 13:34:26.0583 (UTC) FILETIME=[422D9270:01C89E34] X-Scanned-By: MailControl A-08-00-05 (www.mailcontrol.com) on 10.71.0.130 X-Virus-Scanned: ClamAV 0.91.2/6759/Mon Apr 14 14:56:05 2008 on bifrost X-Virus-Status: Clean Vincent Lefevre wrote: > Hi, > > Say that savehistfile is called with err = 0 and that unlink(tmpfile) > fails (hist.c, line 2204). Then it seems that tmpfile will never be > free'd because all the "free(tmpfile);" are in conditions that will > always be false. > > Wouldn't it be simpler to replace all the "free(tmpfile);" by > > if (tmpfile) { > free(tmpfile); > } > > at the end? It does look suspicious. However, we're currently using tmpfile to indicate whether we should be reporting an error about the temporary file, so without more work it looks like we can't actually remove the early frees. They should certainly be setting tmpfile to NULL, though. Index: Src/hist.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/hist.c,v retrieving revision 1.73 diff -u -r1.73 hist.c --- Src/hist.c 11 Mar 2008 10:00:39 -0000 1.73 +++ Src/hist.c 14 Apr 2008 13:31:43 -0000 @@ -2214,6 +2214,7 @@ #endif && sb.st_uid != euid) { free(tmpfile); + tmpfile = NULL; if (err) { if (isset(APPENDHISTORY) || isset(INCAPPENDHISTORY) || isset(SHAREHISTORY)) @@ -2292,6 +2293,7 @@ if (rename(tmpfile, unmeta(fn)) < 0) zerr("can't rename %s.new to $HISTFILE", fn); free(tmpfile); + tmpfile = NULL; } if (writeflags & HFILE_SKIPOLD @@ -2317,12 +2319,13 @@ ret = -1; if (ret < 0 && err) { - if (tmpfile) { + if (tmpfile) zerr("failed to write history file %s.new: %e", fn, errno); - free(tmpfile); - } else + else zerr("failed to write history file %s: %e", fn, errno); } + if (tmpfile) + free(tmpfile); unlockhistfile(fn); } -- Peter Stephenson Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070