On Tue, Jan 15, 2019 at 11:32:56AM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > On Sat, Jan 12, 2019 at 10:51:42AM -0800, Stefan Beller wrote:
> >
> >> > I wonder, and not as "you should do this" feedback on this series, just
> >>
> >> There is a getenv_safe() in environment.c, but I guess a xgetenv() that
> >> takes the same parameters as getenv() is better for ease of use.
> >
> > Yes, but it punts on the memory ownership by stuffing everything into an
> > argv_array. That saves a few lines if you're going to ask for five
> > variables, but for a single variable it's no better than:
> >
> > char *foo = getenv_safe("FOO");
>
> You meant xstrdup_or_null(getenv("FOO")) here? And did Stefan mean
>
> #define xgetenv(e) xstrdup_or_null(getenv(e))
>
> ?
Yes, I think that would be one possible implementation of a "safe"
getenv (and what I was thinking of specifically in that example).
The more involved one (that doesn't pass along memory ownership) is
something like:
static struct hashmap env_cache;
const char *getenv_safe(const char *name)
{
if (e = hashmap_get(&env_cache, name))
return e->value;
/* need some trickery to make sure xstrdup does not call getenv */
e->value = xstrdup_or_null(getenv(name));
e->name = xstrdup(name);
hashmap_put(&env_cache, e);
return e->value;
}
with a matching setenv_safe() to drop the hashmap entry. Come to think
of it, this is really pretty equivalent to string-interning, which we
already have a hashmap for. I think one could argue that string
interning is basically just a controlled form of memory leaking, but
it's probably a reasonable compromise in this instance (i.e., we expect
to ask about a finite number of variables anyway; the important thing is
just that we don't leak memory for the same variable over and over).
-Peff
