Date: Wed, 26 Jan 2000 17:40:02 -0800 (PST) From: "Rodney W. Grimes" <rgrimes@gndrsh.dnsmgr.net> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/10342: putenv(3) unnecessarily calls strdup/free Message-ID: <200001270140.RAA82747@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/10342; it has been noted by GNATS.
From: "Rodney W. Grimes" <rgrimes@gndrsh.dnsmgr.net>
To: peter.jeremy@alcatel.com.au (Peter Jeremy)
Cc: FreeBSD-gnats-submit@FreeBSD.ORG
Subject: Re: bin/10342: putenv(3) unnecessarily calls strdup/free
Date: Wed, 26 Jan 2000 17:34:05 -0800 (PST)
> On 2000-Jan-27 09:19:24 +1100, "Rodney W. Grimes" <rgrimes@gndrsh.dnsmgr.net> wrote:
> >a) This is tried and true tested code that has not been modified
> > since 4.4BSD Lite.
> Accepted. (Though I don't believe that this _automatically_ makes the
> code perfect).
I'll counter with the fact that the code as written is extreamly safe
and independent of specific, and possibly non-standard, implementations
of setenv.
> >b) The man page says ``the given arguments name and value may be appended
> > and prepended, respectively, with an equal sign ``=''.'' It does not
> > say that you can have additional stuff after the =, your patch leaves
> > stuff after the =.
>
> A slight difference in interpretation: I read the manual page as
> meaning that the name argument to setenv() could be _terminated_ with
> either NUL ('\0') or '=' - ie what came after the '=' is irrelevant.
>
> The current implementation of setenv() (and __findenv()) does treat a
> trailing '=' as a terminator. (And I'm not sure how/why someone would
> write code that accepted and ignored '=', but did not treat it as a
> terminator).
Looking at implementation is probably not a good idea, as that may
change. Also depending on behavior not in Posix may cause problems
down the road. What does Posix say about the semantics of any of this,
I see the man page has no posix reference and only getenv is references
to ISO C.
I also have great concerns that this type of auto termination assumption
is leading to things like:
# env
...
XXX=adsfasdf=
...
Now you tell me.... how did I do that, and is the env name
XXX or XXX=asdfasdf and just how is getenv/setenv/putenv going
to deal with this, let alone a program.
[Okay, I'll give you a hint, it was set with csh(1) and here is what
sh(1) does with a few echo's
# echo ${XXX}
adsfasdf=
# echo ${XXX=adsfasdf}
adsfasdf=
#
Can you see why this whole idea of trying to do the = simplification
has lead to a rotting crock of shit??
> >The PR eludes to a memory leak, can you be more specific on exactly how
> >this memory leak occurs as I do not see a memory leak in putenv as the
> >code is now.
>
> An associated PR (bin/10341) discusses a memory leak in setenv()
> (which was previously mentioned in bin/5604). I have no evidence for
> a memory leak within putenv() itself (though my demonstration code for
> bin/10341 uses putenv()). My bin/10342 was solely an efficiency
> improvement (removing calls to strdup/free).
I think Archie just commited the man page changes to document the
leak. Efficiency that can lead to bugs due to reading the implementation
of another part of the code is a bug waiting to happen.
>
> If you are not comfortable with this change, I'll accept your right to
> refuse to apply it. I was just going through my outstanding PR's and
> prodding people over the ones I saw as trivial.
Infact now that you have made me really go look at this, I would like
to ask that you close this PR as ``a bad idea''. Micro optimization
that can lead to one blowing your toe off is not a good thing.
--
Rod Grimes - KD7CAX @ CN85sl - (RWG25) rgrimes@gndrsh.dnsmgr.net
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200001270140.RAA82747>