From owner-freebsd-current@FreeBSD.ORG Fri Apr 20 01:06:36 2007 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A6E7716A402 for ; Fri, 20 Apr 2007 01:06:36 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outO.internet-mail-service.net (outO.internet-mail-service.net [216.240.47.238]) by mx1.freebsd.org (Postfix) with ESMTP id 9121F13C489 for ; Fri, 20 Apr 2007 01:06:36 +0000 (UTC) (envelope-from julian@elischer.org) Received: from mx0.idiom.com (HELO idiom.com) (216.240.32.160) by out.internet-mail-service.net (qpsmtpd/0.32) with ESMTP; Thu, 19 Apr 2007 17:34:39 -0700 Received: from [10.251.22.38] (nat.ironport.com [63.251.108.100]) by idiom.com (Postfix) with ESMTP id 1FB52125AEB; Thu, 19 Apr 2007 18:06:35 -0700 (PDT) Message-ID: <46281223.8060800@elischer.org> Date: Thu, 19 Apr 2007 18:06:43 -0700 From: Julian Elischer User-Agent: Thunderbird 1.5.0.10 (Macintosh/20070221) MIME-Version: 1.0 To: "Sean C. Farley" References: <20070419175902.R44041@thor.farley.org> In-Reply-To: <20070419175902.R44041@thor.farley.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org Subject: Re: Fix for memory leak in setenv/unsetenv (take 2) 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: Fri, 20 Apr 2007 01:06:36 -0000 Sean C. Farley wrote: > I have a new patch[1] that fixes memory leaks caused by repeated calls > to setenv() with varying-sized values or unsetenv(). The web page has a > better description about it. > I vaguely remember that several people have tried to fix this before, but that fixing it actually breaks some ABI.. I may be wrong but maybe others remember better what the issue was. I believe that the end result was that it was considered better to leak memory than break the posix specified ABI in some way. It's very vague in my memory though. > I tested this with -STABLE with various applications. Also, to help > check it further, I wrote a few regression tests for it. > > Notes/questions I have: > 1. Would making it more IEEE Std 1003.1-compliant be desired? > a. FreeBSD's setenv() allows an '=' in the name and value to comply > with other standards while this standard disallows it. > b. FreeBSD's unsetenv() does not have a return value while this > standard returns an int. For the changes I made, this would be > beneficial. > 2. The "feature"--it is under the BUGS section :)--of keeping all > pointers returned by previous calls to getenv() valid regardless of > any calls to setenv() is still there. Keeping this requirement > prevents a complete fix. > 3. I previously thought about having the implementation initialize > itself upon the library loading or called within crt1.c. The problem > with a library-load time method is that changes made to the environ > variable do not persist at the execution of main(). The issue with > calling it within crt1.c is that FreeBSD's malloc() would need to > call __findenv_environ() to find MALLOC_OPTIONS when it is > initialized to prevent a recursion into each other. jasone@ helped > me look at this. > 4. Ignore dmalloc defines for now; they will be removed. > > Basically, the patch contains sysenv.c and a change to the Makefile to > remove building of putenv.c and setenv.c. To increase the speed of > putenv(), I have moved it into sysenv.c to make use of some internal > (static) functions. I have just started with it, but I may leave it > alone. > > Please let me know if you see any bugs. > > Sean > 1. http://www.farley.org/freebsd/tmp/setenv-6/