From owner-freebsd-current@FreeBSD.ORG Thu Oct 19 18:01:53 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9525916A63B for ; Thu, 19 Oct 2006 18:01:53 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id AE97743D49 for ; Thu, 19 Oct 2006 18:01:52 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.6/8.13.6) with ESMTP id k9JI1JFs067884; Thu, 19 Oct 2006 14:01:27 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Bakul Shah Date: Thu, 19 Oct 2006 13:54:08 -0400 User-Agent: KMail/1.9.1 References: <20061019172903.523E15B59@mail.bitblocks.com> In-Reply-To: <20061019172903.523E15B59@mail.bitblocks.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200610191354.09250.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Thu, 19 Oct 2006 14:01:27 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/2050/Thu Oct 19 03:58:33 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: freebsd-current@freebsd.org, "Sean C. Farley" Subject: Re: Fix for memory leak in setenv/unsetenv X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Oct 2006 18:01:53 -0000 On Thursday 19 October 2006 13:29, Bakul Shah wrote: > > I preserve the leak, but I also make use of old entries with matching > > names if their size is big enough. The maximum size of the value is > > recorded at allocation. The code in the source tree uses the strlen() > > of the current value which can shrink even if the space is available. > > > > Example: > > setenv("FOO", "BAR1", 1); > > w = getenv("FOO"); > > setenv("FOO", "BARBAR1", 1); > > x = getenv("FOO"); > > setenv("FOO", "BAR2", 1); > > y = getenv("FOO"); > > setenv("FOO", "BARBAR2", 1); > > z = getenv("FOO"); > > > > This will end up with w == y ("BAR2") and y == z ("BAR1"). w was > > allocated first in the array of variables. x is then allocated since w > > is too small. y finds w within the array and reuses it. z does the > > wame with x. If the larger value had been allocated first, then all > > setenv() calls would have used the same storage space. Yes, there is a > > leak, but flipping back and forth does not cause the leak to keep > > growing. Also, there would be no need to space-pad a value to prevent > > the leak. > > Consider > > > w = getenv("FOO"); > > setenv("FOO", "BARBAR1", 1); > > x = getenv("FOO"); > > The C standard says that the value returned by getenv() shall > *not* be modified. AFAIK it doesn't have setenv(). > > Which leads me to think that in the above example what w > points to *must not* change as a result of a subsequent > setenv() -- w value must be a constant until the program > terminates or another w = getenv(...) is done. Note that > setenv() & getenv() may be called in totally separate > modules, developed by different people at different times. > > You are stuck with a leak (IMHO a small price to pay for > cleaner & standard complying semantics -- you can always use > a conservative GC such as Boehm-Weiser if leak is a major > problem and you care enough). I think this is what John > Baldwin was referring to by "intentional leak". > > Please don't "fix" the leak. Actually, the current setenv() already breaks that rule in that it reuses memory if the current string value is not shorter than the new string value. Sean is just fixing it to use the real length rather than strlen(). -- John Baldwin