From owner-freebsd-current@FreeBSD.ORG Thu Oct 19 02:50:19 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 9956E16A47B; Thu, 19 Oct 2006 02:50:19 +0000 (UTC) (envelope-from sean-freebsd@farley.org) Received: from mail.farley.org (farley.org [67.64.95.201]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1D8A343D58; Thu, 19 Oct 2006 02:50:18 +0000 (GMT) (envelope-from sean-freebsd@farley.org) Received: from dhcp0.farley.org (dhcp0.farley.org [192.168.1.100]) by mail.farley.org (8.13.8/8.13.8) with ESMTP id k9J2pFLt016401; Wed, 18 Oct 2006 21:51:15 -0500 (CDT) (envelope-from sean-freebsd@farley.org) Date: Wed, 18 Oct 2006 21:50:26 -0500 (CDT) From: "Sean C. Farley" To: John Baldwin In-Reply-To: <200610111427.42195.jhb@freebsd.org> Message-ID: <20061018211005.L1466@baba.farley.org> References: <20061006200320.T1063@baba.farley.org> <200610101001.04286.jhb@freebsd.org> <20061011105435.A10713@thor.farley.org> <200610111427.42195.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-1563628599-1161226226=:1466" Cc: freebsd-current@freebsd.org 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 02:50:19 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-1563628599-1161226226=:1466 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed On Wed, 11 Oct 2006, John Baldwin wrote: > On Wednesday 11 October 2006 12:15, Sean C. Farley wrote: >> On Tue, 10 Oct 2006, John Baldwin wrote: >>> This still won't work. The reason for the intentional leak is >>> because of this code sequence: >>> >>> char *a; >>> >>> setenv("FOO", "0", 1); >>> a = getenv("FOO"); >>> setenv("FOO", "bar", 1); >>> printf("FOO was %s\n", a); >>> >>> With the memory leak fixed this will use free'd memory. While this >>> code may seem weird in a program, it actually is quite possible for >>> a library to read and cache the value of an environment variable. >>> If you didn't leave the leak around, the library could cause a crash >>> if the main program (or another library) changed the environment >>> variable the first library had a cached pointer to the value of. > Yeah, but it doesn't crash is the point actually. The pointer is > still valid, though it may be overwritten with a newer value, it's > still valid and a library can reliably doing getenv() and that pointer > will always point to some value of that variable, but it won't ever > point to anything else. > Part of the problem is that we have no way to notify consumers of an > environment variable when its value is changed. Alternatively, we > could add a different variant of getenv that required the user to > supply the buffer, but that's not the API we have. OK. I decided to fix the memory leak as well as keep backward compatibility. The result is on my site tar'd[1] and extracted[2]. It still needs some touch-ups, but it works. It is even faster than the current implementation when I compared "hungry" and "lean" (main.c without the sleep() call). Features: 1. No memory leak. :) 2. Able to be cleaned up: __clean_environ() 3. Backward compatible. 4. Faster. Nice changes to make (but unnecessary): 1. Call __build_env() just after a process starts instead of checking within the public env functions. 2. Call __rebuild_environ() just before uses of environ within libc (i.e., execl(), execv(), execvP(), popen(), _init_tls()) instead of after every getenv(), setenv(), unsetenv(). 3. Call __clean_environ() at process exit. This allows for fewer leaks when a developer is debugging his code. Sean 1. http://www.farley.org/freebsd/tmp/setenv-4.tar.bz2 2. http://www.farley.org/freebsd/tmp/setenv-4/ -- sean-freebsd@farley.org --0-1563628599-1161226226=:1466 Content-Type: TEXT/PLAIN; charset=US-ASCII; name=logging Content-Transfer-Encoding: BASE64 Content-ID: <20061018215026.F1466@baba.farley.org> Content-Description: ministat log Content-Disposition: attachment; filename=logging eCAuLi9odW5ncnkubG9nDQorIC4uL2xlYW4ubG9nDQorLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0rDQp8ICArICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB8DQp8ICsrICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8DQp8Kysr ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgIHggeCB4ICB8DQp8KysrICAgKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgeHh4eHh4IHh8DQp8fE1BfCAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHxfQV98 ICB8DQorLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0rDQogICAgTiAg ICAgICAgICAgTWluICAgICAgICAgICBNYXggICAgICAgIE1lZGlhbiAgICAg ICAgICAgQXZnICAgICAgICBTdGRkZXYNCnggIDEwICAgICAgICAgIDIuMjIg ICAgICAgICAgMi4yOSAgICAgICAgICAyLjI1ICAgICAgICAgMi4yNTEgICAw LjAyMTgzMjY5Nw0KKyAgMTAgICAgICAgICAgMS41MyAgICAgICAgICAxLjU5 ICAgICAgICAgMS41NDUgICAgICAgICAxLjU0NyAgIDAuMDE3MDI5Mzg2DQpE aWZmZXJlbmNlIGF0IDk1LjAlIGNvbmZpZGVuY2UNCgktMC43MDQgKy8tIDAu MDE4Mzk2Mw0KCS0zMS4yNzUlICsvLSAwLjgxNzI0OCUNCgkoU3R1ZGVudCdz IHQsIHBvb2xlZCBzID0gMC4wMTk1Nzg5KQ0K --0-1563628599-1161226226=:1466--