git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Jan 15, 2019, 19:47 UTC
Message-ID
<20190115194726.GA5818@sigill.intra.peff.net>
In-Reply-To
<20190115194142.GG4886@sigill.intra.peff.net>
On Tue, Jan 15, 2019 at 02:41:42PM -0500, Jeff King wrote:
Show 26 quoted lines
> 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).

So actually, that's pretty easy to do without writing much code at all. Something like:

  #define xgetenv(name) strintern(getenv(name))

It means we're effectively storing the environment twice in the worst case, but that's probably not a big deal. Unless we have a loop which does repeated setenv()/getenv() calls, the strintern hashmap can't grow without bound.

-Peff
Previous: Jeff KingNext: Junio C Hamano
Message 24 of 26 in “getenv() timing fixes”
  1. 0/6 getenv() timing fixesJeff King, Jan 11, 2019
  2. 1/6 get_super_prefix(): copy getenv() resultJeff King, Jan 11, 2019
  3. Junio C HamanoJan 12, 2019
  4. 2/6 commit: copy saved getenv() resultJeff King, Jan 11, 2019
  5. Junio C HamanoJan 12, 2019
  6. Jeff KingJan 12, 2019
  7. Johannes SchindelinJan 15, 2019
  8. Jeff KingJan 15, 2019
  9. Stefan BellerJan 15, 2019
  10. Jeff KingJan 15, 2019
  11. Johannes SchindelinJan 16, 2019
  12. 3/6 config: make a copy of $GIT_CONFIG stringJeff King, Jan 11, 2019
  13. 4/6 init: make a copy of $GIT_DIR stringJeff King, Jan 11, 2019
  14. Junio C HamanoJan 12, 2019
  15. 5/6 merge-recursive: copy $GITHEAD stringsJeff King, Jan 11, 2019
  16. Junio C HamanoJan 12, 2019
  17. 6/6 builtin_diff(): read $GIT_DIFF_OPTS closer to useJeff King, Jan 11, 2019
  18. Ævar Arnfjörð BjarmasonJan 12, 2019
  19. Stefan BellerJan 12, 2019
  20. Jeff KingJan 15, 2019
  21. Junio C HamanoJan 15, 2019
  22. Stefan BellerJan 15, 2019
  23. Jeff KingJan 15, 2019
  24. Jeff KingJan 15, 2019
  25. Junio C HamanoJan 15, 2019
  26. Jeff KingJan 15, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.