Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 8 Aug 1998 10:42:59 +0300 (EEST)
From:      Narvi <narvi@haldjas.folklore.ee>
To:        Garance A Drosihn <drosih@rpi.edu>
Cc:        Archie Cobbs <archie@whistle.com>, Garrett Wollman <wollman@khavrinen.lcs.mit.edu>, freebsd-current@FreeBSD.ORG
Subject:   Re: memory leaks in libc
Message-ID:  <Pine.BSF.3.96.980808103541.9173J-100000@haldjas.folklore.ee>
In-Reply-To: <v04011702b1f10c76c658@[128.113.24.47]>

next in thread | previous in thread | raw e-mail | index | archive | help

On Fri, 7 Aug 1998, Garance A Drosihn wrote:

> At 10:12 AM -0700 8/7/98, Archie Cobbs wrote:
> >Garrett Wollman writes:
> >> Now having gone to that effort, you can just add it to your program
> >> that needs it, and we don't have to bear the kluge in the C library.
> >
> > I just don't understand where you're coming from. There is a clear
> > bug in the standard library, we agree on this right? You are saying
> > that it's not worth fixing, because...
> 
> I don't know where Garrett is coming from, but I must admit I would
> be leary of making changes to getenv/setenv.  The main reason is
> that the environment mechanism was poorly thought out and defined
> to begin with, and thus any changes you make are likely to break
> something somewhere.  By "poorly defined", I mean that there are
> not good, consistent definitions of what one can and can not expect
> from a call to getenv/putenv, and thus you can be sure that some
> code uses those routines in ways you won't forsee.
> 
> Consider that getenv returns a pointer to the environment variable.
> It does not malloc any space, it just returns a pointer to the
> actual value it finds in the current environment.  Now consider
> the following sequence:
>      setenv("NAME","VAL1");    -> does a malloc to store NAME
>      keep = getenv("NAME");    -> returns a pointer to the middle
>                                   of that malloc'ed area
>      setenv("NAME","VAL2");    -> replaces the value of NAME as
>                                   is kept in the environment
>      if ( ! strcmp(keep, "VAL1") ) { do_stuff(); }
> 
>           -> Here is the problem.  In the above sequence, you
>           -> "know" that setenv malloc'ed the space, and thus
>           -> you would expect that the second setenv should
>           -> free it.  However, the program has stored away
>           -> a pointer into that area which it got from getenv,
>           -> and it has every right to believe (due to years of
>           -> precedent) that that pointer will remain valid.
>           -> if you free and reuse that space, you may introduce
>           -> some pretty obscure bugs.
> 

Actually, *if* strlen(VAL2) > strlen(VAL1) even the current code uses
realloc, which might relocate the array.

And the proposed replacement to the current code would not change this.

Neither would it break this case you brought up, if reallocation does not
take place. 

> And *that* is in the "well-understood" case, were we know that
> the only things playing with the environment have been setenv
> and getenv.  However, there are plenty of programs which
> manipulate the environment via other means, and would make
> life even more complicated.
> 

Every program that references potencially freed storage is broken.

> If the question is:
>    Can you design a safe *alternative* to setenv/getenv which
>    would not have this memory-leak side-effect?

The answer is yes. See above.

> the answer is definitely yes.  However, for the specific routines
> of getenv/setenv/putenv, the horses left the barn a long time ago.
> There's no sense trying to lock the door now.  I think that the
> risks of *replacing* getenv/setenv/putenv with a "smarter" version
> are far greater than the benefits of fixing this memory leak.
> 

At the worst, we could have a variable like in the case of phkmalloc...

The right way, of course, is fixing the real bogus programs.

> If you wanted to define some "getenv2/setenv2", which worked on a
> vector called **environ2 (and *not* the standard, "well-known"
> environment vector), then there's all kinds of good things you
> can do.  However, in my opinion replacing getenv/setenv is much

There's another good idea.

> too risky to be worth it.  If you find some stupid program which
> really does have a serious memory leak due to setenv calls, then
> fix that one program.  The memory leak would be easy enough to
> find and to fix in a program, as opposed to introducing very
> non-obvious bugs to a hole slew of programs which are currently
> working just fine as they are (even if they do occasionally "leak"
> 64 bytes of memory somewhere, nothing is the worse for that).
> 
> So, I think Garrett's position is sensible and corrrect, even if
> his presentation could be somewhat more pleasent or informative.
> 
> ---
> Garance Alistair Drosehn           =   gad@eclipse.its.rpi.edu
> Senior Systems Programmer          or  drosih@rpi.edu
> Rensselaer Polytechnic Institute
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message
> 

	Sander

	There is no love, no good, no happiness and no future -
	all these are just illusions.




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.3.96.980808103541.9173J-100000>