Subject: Concerns about SetEnv

From: Phil Pennock <[email-masked]>
Date: Tue, 20 Jun 2000 18:50:10 +0200

Hi,

I saw the SetEnv function in the manual-page, which was a new one on me
(I stopped keeping up with fvwm2 changes years ago).  I wanted to
explicitly unset a variable ($SHLVL) and to test this I:
 * Started FvwmTalk
 * SetEnv SHLVL 3
 * Exec exec xterm
and then tried various methods of unsetting.  When none seemed to work
(aside from giving it value 0) I went to look at the source.

I'm concerned at this code.  It appears to be a memory leak.
putenv() doesn't stick a pointer to the string straight in environ - it
allocates the memory itself, handling cases where you set a variable
twice, etc.  So the szPutenv is leaked.  If there are systems (which
AIUI would be broken) where putenv() does not allocate the memory, then
there is a memory-leak where you run SetEnv twice on the same variable
name.

Looking (admittedly not very hard) at the code, I can't see any way to
unset an environment variable which has previously been set.  Am I
missing something here, or does Fvwm need an UnSetEnv command too?

(Not on list, please CC)

Thanks,
-- 
Phil Pennock
"We've got a patent on the conquering of a country through the use of force.
 We believe in world peace through extortionate license fees."  -Bluemeat
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: [email-masked] (Paul D. Smith)
Date: 20 Jun 2000 13:36:08 -0400

%% Phil Pennock <[email-masked]> writes:

  pp> I'm concerned at this code.  It appears to be a memory leak.

There is a known leak here, yes.

  pp> putenv() doesn't stick a pointer to the string straight in environ
  pp> - it allocates the memory itself, handling cases where you set a
  pp> variable twice, etc.  So the szPutenv is leaked.

What OS are you using?  That's not true on any UNIX system I've used.
The memory you pass to putenv() is installed directly into the
environment.  If you change that memory, you change the environment.  If
you free that memory, you corrupt the environment.

This is true on AIX, HP-UX, Solaris, SunOS, Linux, FreeBSD, and is
required by the SingleUNIX standard (putenv isn't in POSIX).  The man
page sez:

 DESCRIPTION
     putenv() makes the value of the environment  variable  name  equal  to
     value  by  altering  an  existing  variable or creating a new one.  In
     either case, the string pointed to  by  string  becomes  part  of  the
     environment,  so  altering  the  string  will  change the environment.
     string points to a string of the form ``name=value.''  The space  used
     by  string is no longer used once a new string-defining name is passed
     to putenv().

That use of putenv() is not a leak.

  pp> If there are systems (which AIUI would be broken) where putenv()
  pp> does not allocate the memory, then there is a memory-leak where
  pp> you run SetEnv twice on the same variable name.

How do you know what to free?  One way you might consider is using
getenv() before you try the putenv() to see if there's any previous
value.  The problem with this is that it's not clear that you're allowed
to free any value you get back from getenv()... what if that value was
included in your invocation environment, and is part of a larger, single
allocated block of environment values?

The only way to be sure would be to keep track of the memory you've
provided to putenv() separately, and that's just too much overhead to
bother with given the very minor amount of memory leakage we're talking
about here.  So, we didn't :).

  pp> Looking (admittedly not very hard) at the code, I can't see any
  pp> way to unset an environment variable which has previously been
  pp> set.  Am I missing something here, or does Fvwm need an UnSetEnv
  pp> command too?

I don't know if "need" is the right term (I don't recall anyone ever
wanting one before) but I don't see why it couldn't be implemented.

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <[email-masked]>         Network Management Development
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
-------------------------------------------------------------------------------
   These are my opinions---Nortel Networks takes no responsibility for them.
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Phil Pennock <[email-masked]>
Date: Tue, 20 Jun 2000 20:05:45 +0200

> This is true on AIX, HP-UX, Solaris, SunOS, Linux, FreeBSD, and is

FreeBSD3.4 and OpenBSD2.5 both; probably then anything BSD4.4 derived.

putenv(1):
  The putenv() function takes an argument of the form ``name=value''
  and is equivalent to:
    setenv(name, value, 1);
and then reading /usr/src/lib/libc/stdlib/setenv.c (same file, slightly
differing code).

Hrm, maybe I've been doing too much sysadmin/perl, but to me it looks in
this code like stdlib setenv()/unsetenv() leak.  :^(

In fact, a quick test confirms that they do.  sendbug time ...

>      by  string is no longer used once a new string-defining name is passed

Ah well, SingleUNIX doesn't seem to match BSD4.4 reality then.

> That use of putenv() is not a leak.

On the systems described above, it is.  On systems which match the spec,
it is, as you describe.

> about here.  So, we didn't :).

Okay.  Is this a documented buglet, then?

> wanting one before) but I don't see why it couldn't be implemented.

:^)  Ta.
-- 
HTML email - just say no --> Phil Pennock