Re: [PATCH 0/6] getenv() timing fixes

From: Jeff King <peff@peff.net>
Date: Jan 15, 2019, 19:41 UTC
Message-ID: <20190115194142.GG4886@sigill.intra.peff.net>
In-Reply-To: <xmqqy37lra1j.fsf@gitster-ct.c.googlers.com>

On Tue, Jan 15, 2019 at 11:32:56AM -0800, Junio C Hamano wrote:

Show 20 quoted lines

> Jeff King <peff@peff.net> 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