[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Mon, 21 Mar 2016 15:45:23 +0300 (MSK)
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/3] overhaul environment functions
On Sun, 20 Mar 2016, Alexander Monakov wrote:
> On Sun, 20 Mar 2016, Rich Felker wrote:
> > After debunking most of my concerns, are there any things left to
> > change in patch 1, or should I go ahead and apply it?
>
> Let me send a v2 with two changes: reinstating setenv EINVAL on null arg,
> and tweaking unsetenv like I mentioned. Should be good with those changes,
> I think.
I've attempted another go at testing, and it turns out the new code still has
a memory leak, although a subtler and smaller one. It is caused by failure to
use existing null slots in __env_change when it is called with p != 0, but no
slot in env_alloced matches p (so the early-out by replacing p by r and
freeing p does not happen).
I'm quite surprised I didn't catch this sooner, because the testcase for the
existing leak (alternating setenv/putenv calls) also exposes this one, albeit
at a slower rate. I've probably made some mistake like misinterpreting the
exit status; not sure.
The following incremental diff shows how I have addressed this issue. I'm
leaving it for comments, and will send an updated patch with two changes
mentioned above, plus __env_change renamed to __env_rm_add, some time later.
Alexander
diff --git a/src/env/setenv.c b/src/env/setenv.c
index db9d72a..051c8d0 100644
--- a/src/env/setenv.c
+++ b/src/env/setenv.c
@@ -9,17 +9,23 @@ void __env_change(char *p, char *r)
{
static char **env_alloced;
static size_t env_alloced_n;
+ char **nullslot = 0;
for (size_t i=0; i < env_alloced_n; i++)
if (env_alloced[i] == p) {
env_alloced[i] = r;
free(p);
return;
+ } else if (!env_alloced[i]) {
+ nullslot = env_alloced + i;
}
if (!r) return;
- char **new_ea = realloc(env_alloced, sizeof *new_ea * (env_alloced_n+1));
- if (!new_ea) return;
- new_ea[env_alloced_n++] = r;
- env_alloced = new_ea;
+ if (!nullslot) {
+ char **t = realloc(env_alloced, sizeof *t * (env_alloced_n+1));
+ if (!t) return;
+ env_alloced = t;
+ nullslot = env_alloced + env_alloced_n++;
+ }
+ *nullslot = r;
}
int setenv(const char *var, const char *value, int overwrite)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
