From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39298) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTWWf-0004EP-Pv for qemu-devel@nongnu.org; Thu, 24 Nov 2011 05:27:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTWWa-0003Nx-7T for qemu-devel@nongnu.org; Thu, 24 Nov 2011 05:27:29 -0500 Date: Thu, 24 Nov 2011 10:27:12 +0000 From: Stefan Hajnoczi Message-ID: <20111124102712.GA27170@stefanha-thinkpad.localdomain> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] os-win32.c : fix memory leak List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cc: qemu-trivial@nongnu.org, Zhi Hui Li , qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On Thu, Nov 24, 2011 at 05:15:30PM +0800, Mark wrote: > If you free the string, it will cause the environment variable unavailable. > More details please see the following text extracted from manual of > "putenv": > > The libc4 and libc5 and glibc 2.1.2 versions conform to SUSv2: > the pointer string given to putenv() is used. In particular, this > string becomes part of the environment; changing it later will > change the environment. (Thus, it is an error is to call putenv() with > an automatic variable as the argument, then return from the calling > function while string is still part of the environment.) However, > glibc 2.0-2.1.1 differs: a copy of the string is used. On the one > hand this causes a memory leak, and on the other hand it violates > SUSv2. This has been fixed in glibc 2.1.2. I don't think this matters since os-win32.c is only built for mingw, which uses the Microsoft C runtime and not glibc. However, there is no documentation for putenv(3) on MSDN because the function has been deprecated :(. So I think the safest thing to do is to assume this will leak memory but we are not allowed to free the string. Either you could investigate the new _putenv(3) and test the Windows build to make sure it works. Or you could send a patch that adds a comment explaining why there is a memory leak here. Stefan