Skip to content

Commit

Permalink
Significantly reduce the memory leak as noted in BUGS section for
Browse files Browse the repository at this point in the history
setenv(3) by tracking the size of the memory allocated instead of using
strlen() on the current value.

Convert all calls to POSIX from historic BSD API:
 - unsetenv returns an int.
 - putenv takes a char * instead of const char *.
 - putenv no longer makes a copy of the input string.
 - errno is set appropriately for POSIX.  Exceptions involve bad environ
   variable and internal initialization code.  These both set errno to
   EFAULT.

Several patches to base utilities to handle the POSIX changes from
Andrey Chernov's previous commit.  A few I re-wrote to use setenv()
instead of putenv().

New regression module for tools/regression/environ to test these
functions.  It also can be used to test the performance.

Bump __FreeBSD_version to 700050 due to API change.

PR:		kern/99826
Approved by:	wes
Approved by:	re (kensmith)
  • Loading branch information
scf authored and scf committed Jul 4, 2007
1 parent af5bbfb commit 196b634
Show file tree
Hide file tree
Showing 28 changed files with 1,389 additions and 290 deletions.
8 changes: 4 additions & 4 deletions bin/df/df.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,14 @@ main(int argc, char *argv[])
*/
if (kflag)
break;
putenv("BLOCKSIZE=512");
setenv("BLOCKSIZE", "512", 1);
hflag = 0;
break;
case 'c':
cflag = 1;
break;
case 'g':
putenv("BLOCKSIZE=1g");
setenv("BLOCKSIZE", "1g", 1);
hflag = 0;
break;
case 'H':
Expand All @@ -152,7 +152,7 @@ main(int argc, char *argv[])
break;
case 'k':
kflag++;
putenv("BLOCKSIZE=1024");
setenv("BLOCKSIZE", "1024", 1);
hflag = 0;
break;
case 'l':
Expand All @@ -162,7 +162,7 @@ main(int argc, char *argv[])
lflag = 1;
break;
case 'm':
putenv("BLOCKSIZE=1m");
setenv("BLOCKSIZE", "1m", 1);
hflag = 0;
break;
case 'n':
Expand Down
16 changes: 13 additions & 3 deletions bin/sh/var.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ void
setvareq(char *s, int flags)
{
struct var *vp, **vpp;
char *p;
int len;

if (aflag)
Expand Down Expand Up @@ -319,7 +320,10 @@ setvareq(char *s, int flags)
if (vp == &vmpath || (vp == &vmail && ! mpathset()))
chkmail(1);
if ((vp->flags & VEXPORT) && localevar(s)) {
putenv(s);
p = strchr(s, '=');
*p = '\0';
(void) setenv(s, p + 1, 1);
*p = '=';
(void) setlocale(LC_ALL, "");
}
INTON;
Expand All @@ -335,7 +339,10 @@ setvareq(char *s, int flags)
INTOFF;
*vpp = vp;
if ((vp->flags & VEXPORT) && localevar(s)) {
putenv(s);
p = strchr(s, '=');
*p = '\0';
(void) setenv(s, p + 1, 1);
*p = '=';
(void) setlocale(LC_ALL, "");
}
INTON;
Expand Down Expand Up @@ -596,7 +603,10 @@ exportcmd(int argc, char **argv)

vp->flags |= flag;
if ((vp->flags & VEXPORT) && localevar(vp->text)) {
putenv(vp->text);
p = strchr(vp->text, '=');
*p = '\0';
(void) setenv(vp->text, p + 1, 1);
*p = '=';
(void) setlocale(LC_ALL, "");
}
goto found;
Expand Down
4 changes: 2 additions & 2 deletions include/stdlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ void _Exit(int) __dead2;
int posix_memalign(void **, size_t, size_t); /* (ADV) */
int rand_r(unsigned *); /* (TSF) */
int setenv(const char *, const char *, int);
void unsetenv(const char *);
int unsetenv(const char *);
#endif

/*
Expand Down Expand Up @@ -197,7 +197,7 @@ long mrand48(void);
long nrand48(unsigned short[3]);
int posix_openpt(int);
char *ptsname(int);
int putenv(const char *);
int putenv(char *);
long random(void);
char *realpath(const char *, char resolved_path[]);
unsigned short
Expand Down
8 changes: 4 additions & 4 deletions lib/libc/stdlib/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ MISRCS+=_Exit.c a64l.c abort.c abs.c atexit.c atof.c atoi.c atol.c atoll.c \
bsearch.c div.c exit.c getenv.c getopt.c getopt_long.c \
getsubopt.c grantpt.c hcreate.c heapsort.c imaxabs.c imaxdiv.c \
insque.c l64a.c labs.c ldiv.c llabs.c lldiv.c lsearch.c malloc.c \
merge.c putenv.c qsort.c qsort_r.c radixsort.c rand.c random.c \
reallocf.c realpath.c remque.c setenv.c strfmon.c strtoimax.c \
strtol.c strtoll.c strtoq.c strtoul.c strtonum.c strtoull.c strtoumax.c \
strtouq.c system.c tdelete.c tfind.c tsearch.c twalk.c
merge.c qsort.c qsort_r.c radixsort.c rand.c random.c \
reallocf.c realpath.c remque.c strfmon.c strtoimax.c \
strtol.c strtoll.c strtoq.c strtoul.c strtonum.c strtoull.c \
strtoumax.c strtouq.c system.c tdelete.c tfind.c tsearch.c twalk.c

SYM_MAPS+= ${.CURDIR}/stdlib/Symbol.map

Expand Down
104 changes: 80 additions & 24 deletions lib/libc/stdlib/getenv.3
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
.\" @(#)getenv.3 8.2 (Berkeley) 12/11/93
.\" $FreeBSD$
.\"
.Dd October 12, 2006
.Dd June 20, 2007
.Dt GETENV 3
.Os
.Sh NAME
Expand All @@ -50,22 +50,13 @@
.Ft int
.Fn setenv "const char *name" "const char *value" "int overwrite"
.Ft int
.Fn putenv "const char *string"
.Ft void
.Fn putenv "char *string"
.Ft int
.Fn unsetenv "const char *name"
.Sh DESCRIPTION
These functions set, unset and fetch environment variables from the
host
.Em environment list .
For compatibility with differing environment conventions,
the given arguments
.Fa name
and
.Fa value
may be appended and prepended,
respectively,
with an equal sign
.Dq Li \&= .
.Pp
The
.Fn getenv
Expand Down Expand Up @@ -97,11 +88,18 @@ to the given
.Pp
The
.Fn putenv
function takes an argument of the form ``name=value'' and is
equivalent to:
.Bd -literal -offset indent
setenv(name, value, 1);
.Ed
function takes an argument of the form ``name=value'' and
puts it directly into the current environment,
so altering the argument shall change the environment.
If the variable
.Fa name
does not exist in the list,
it is inserted with the given
.Fa value .
If the variable
.Fa name
does exist, it is reset to the given
.Fa value .
.Pp
The
.Fn unsetenv
Expand All @@ -121,15 +119,55 @@ is not in the current environment,
.Dv NULL
is returned.
.Pp
.Rv -std setenv putenv
.Rv -std setenv putenv unsetenv
.Sh ERRORS
.Bl -tag -width Er
.It Bq Er ENOMEM
.It Bq Er EINVAL
The function
.Fn getenv ,
.Fn setenv
or
.Fn unsetenv
failed because the
.Fa name
is a
.Dv NULL
pointer, points to an empty string, or points to a string containing an
.Dq Li \&=
character.
.Pp
The function
.Fn putenv
failed because
.Fa string
is a
.Dv NULL
pointer,
.Fa string is without an
.Dq Li \&=
character or
.Dq Li \&=
is the first character in
.Fa string .
This does not follow the
.Tn POSIX
specification.
.It Bq Er ENOMEM
The function
.Fn setenv ,
.Fn unsetenv
or
.Fn putenv
failed because they were unable to allocate memory for the environment.
.It Bq Er EFAULT
The functions
.Fn setenv ,
.Fn unsetenv
or
.Fn putenv
failed to make a valid copy of the environment due to the environment being
corrupt (i.e., a name without a value). A warning will be output to stderr with
information about the issue.
.El
.Sh SEE ALSO
.Xr csh 1 ,
Expand All @@ -141,6 +179,13 @@ The
.Fn getenv
function conforms to
.St -isoC .
The
.Fn setenv ,
.Fn putenv
and
.Fn unsetenv
functions conforms to
.St -p1003.1-2001 .
.Sh HISTORY
The functions
.Fn setenv
Expand All @@ -152,19 +197,30 @@ The
.Fn putenv
function appeared in
.Bx 4.3 Reno .
.Pp
Until
.Fx 7.0 ,
.Fn putenv
would make a copy of
.Fa string
and insert it into the environment using
.Fn setenv .
This was changed to use
.Fa string
as the memory location of the ``name=value'' pair to follow the
.Tn POSIX
specification.
.Sh BUGS
Successive calls to
.Fn setenv
or
.Fn putenv
assigning a differently sized
that assign a larger-sized
.Fa value
to the same
than any previous value to the same
.Fa name
will result in a memory leak.
The
.Fx
semantics for these functions
semantics for this function
(namely, that the contents of
.Fa value
are copied and that old values remain accessible indefinitely) make this
Expand Down
Loading

0 comments on commit 196b634

Please sign in to comment.