From owner-freebsd-current@FreeBSD.ORG Thu Apr 26 21:48:05 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 D373916A474 for ; Thu, 26 Apr 2007 21:48:05 +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 6176113C4C9 for ; Thu, 26 Apr 2007 21:48:05 +0000 (UTC) (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.8/8.13.8) with ESMTP id l3QLlapc078120; Thu, 26 Apr 2007 17:47:46 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-current@freebsd.org Date: Thu, 26 Apr 2007 17:38:19 -0400 User-Agent: KMail/1.9.6 References: <20070419175902.R44041@thor.farley.org> <46281223.8060800@elischer.org> In-Reply-To: <46281223.8060800@elischer.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200704261738.20399.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, 26 Apr 2007 17:47:47 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/3165/Thu Apr 26 09:03:24 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Julian Elischer , "Sean C. Farley" 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: Thu, 26 Apr 2007 21:48:05 -0000 On Thursday 19 April 2007 09:06:43 pm Julian Elischer wrote: > 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. Sean's changes don't break the ABI, but they do make more efficient use of the memory. Specifically, setenv() will reuse the variable part of the name=value pair if the new value will fit in the same or less space as the old value, and it uses strlen() to determine this. What Sean's patches fix is this case: setenv("FOO", "88888888"); /* 8 char string as value */ setenv("FOO", "4444"); /* 4 chars, reuses 8 char buffer */ setenv("FOO", "88888888"); /* setenv thinks buffer is 4 char, so allocates a new one for 8 char */ Sean keeps track of the true allocated length of each string, so with his change, you the 3rd setenv will reuse the existing buffer instead of allocating a new one. At my previous job, we space padded timezone values out to a fixed value to workaround the problem of alternating between different length timezone names and thus leaking memory endlessly. Since setenv(3) already overwrites the value with a new one in some cases, Sean's patch doesn't change behavior, it just allows it to reuse the old buffer more often than it does now. This really should go in, I just haven't had time to review it in detail. If someone else wants to help and step up to the plate to do that, that would be appreciated. :) -- John Baldwin