From: Stefan Weil <sw@weilnetz.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-trivial@nongnu.org, Mark <wudx05@gmail.com>,
Zhi Hui Li <zhihuili@linux.vnet.ibm.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] os-win32.c : fix memory leak
Date: Thu, 24 Nov 2011 21:46:42 +0100 [thread overview]
Message-ID: <4ECEAD32.3070402@weilnetz.de> (raw)
In-Reply-To: <4ECE9AED.3070209@weilnetz.de>
Am 24.11.2011 20:28, schrieb Stefan Weil:
> Am 24.11.2011 11:27, schrieb Stefan Hajnoczi:
>> 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.
>
> MS claims that putenv is a POSIX function, so I also expected
> that free / f_free is not allowed.
>
> I now wrote a short test which indicates that g_free would work:
> getenv returns a pointer which is completely different from
> the one passed to putenv.
>
> Nevertheless, there is a better solution using _putenv_s.
> I'll send a patch.
>
> Regards,
> Stefan Weil
>
Hi Stefan,
I'm afraid I was too fast when I promised a patch with _putenv_s.
Function _putenv_s is a good solution, but only if it is supported
by MinGW. Older versions of MinGW (Debian Squeeze!) don't support
it :-(
Therefore I suggest to apply this patch. I hope that my test which was
run on XP (32 bit) is sufficient.
Cheers,
Stefan W.
Tested-by: Stefan Weil <sw@weilnetz.de>
WARNING: multiple messages have this Message-ID (diff)
From: Stefan Weil <sw@weilnetz.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-trivial@nongnu.org, Mark <wudx05@gmail.com>,
Zhi Hui Li <zhihuili@linux.vnet.ibm.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] os-win32.c : fix memory leak
Date: Thu, 24 Nov 2011 21:46:42 +0100 [thread overview]
Message-ID: <4ECEAD32.3070402@weilnetz.de> (raw)
In-Reply-To: <4ECE9AED.3070209@weilnetz.de>
Am 24.11.2011 20:28, schrieb Stefan Weil:
> Am 24.11.2011 11:27, schrieb Stefan Hajnoczi:
>> 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.
>
> MS claims that putenv is a POSIX function, so I also expected
> that free / f_free is not allowed.
>
> I now wrote a short test which indicates that g_free would work:
> getenv returns a pointer which is completely different from
> the one passed to putenv.
>
> Nevertheless, there is a better solution using _putenv_s.
> I'll send a patch.
>
> Regards,
> Stefan Weil
>
Hi Stefan,
I'm afraid I was too fast when I promised a patch with _putenv_s.
Function _putenv_s is a good solution, but only if it is supported
by MinGW. Older versions of MinGW (Debian Squeeze!) don't support
it :-(
Therefore I suggest to apply this patch. I hope that my test which was
run on XP (32 bit) is sufficient.
Cheers,
Stefan W.
Tested-by: Stefan Weil <sw@weilnetz.de>
next prev parent reply other threads:[~2011-11-24 20:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-24 8:27 [Qemu-trivial] [PATCH] os-win32.c : fix memory leak Zhi Hui Li
2011-11-24 8:27 ` [Qemu-devel] " Zhi Hui Li
2011-11-24 9:15 ` [Qemu-trivial] " Mark
2011-11-24 9:15 ` Mark
2011-11-24 10:27 ` [Qemu-trivial] " Stefan Hajnoczi
2011-11-24 10:27 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
2011-11-24 19:28 ` [Qemu-trivial] [Qemu-devel] " Stefan Weil
2011-11-24 19:28 ` [Qemu-devel] [Qemu-trivial] " Stefan Weil
2011-11-24 20:46 ` Stefan Weil [this message]
2011-11-24 20:46 ` Stefan Weil
2011-11-25 8:56 ` Paolo Bonzini
2011-11-25 8:56 ` [Qemu-devel] " Paolo Bonzini
2011-11-28 10:49 ` Stefan Hajnoczi
2011-11-28 10:49 ` [Qemu-devel] " Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ECEAD32.3070402@weilnetz.de \
--to=sw@weilnetz.de \
--cc=qemu-devel@nongnu.org \
--cc=qemu-trivial@nongnu.org \
--cc=stefanha@gmail.com \
--cc=wudx05@gmail.com \
--cc=zhihuili@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.