From 96db775ac1ff5df64957d1a9873f0955c3d4cb17 Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Thu, 30 May 2019 02:51:15 +0200
Subject: [PATCH] LibC: Add setenv().

If I'm understanding the standard C library correctly, setenv() copies while
putenv() does not. That's really confusing and putenv() basically sucks.
To know which environment variables to free on replacement and which ones to
leave alone, we keep track of the ones malloced by setenv in a side table.

This patch also moves Shell to using setenv() instead of putenv().

Fixes #29.
---
 LibC/stdlib.cpp | 24 ++++++++++++++++++++++++
 LibC/stdlib.h   |  1 +
 Shell/main.cpp  | 12 ++----------
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/LibC/stdlib.cpp b/LibC/stdlib.cpp
index cabcda573da..74d7d07939d 100644
--- a/LibC/stdlib.cpp
+++ b/LibC/stdlib.cpp
@@ -46,6 +46,16 @@ void abort()
     raise(SIGABRT);
 }
 
+static HashTable<char*> s_malloced_environment_variables;
+
+static void free_environment_variable_if_needed(const char* var)
+{
+    if (!s_malloced_environment_variables.contains((char*)var))
+        return;
+    free((void*)var);
+    s_malloced_environment_variables.remove((char*)var);
+}
+
 char* getenv(const char* name)
 {
     size_t vl = strlen(name);
@@ -89,9 +99,22 @@ int unsetenv(const char* name)
     // Shuffle the existing array down by one.
     memmove(&environ[skip], &environ[skip+1], ((environ_size-1)-skip) * sizeof(environ[0]));
     environ[environ_size-1] = nullptr;
+
+    free_environment_variable_if_needed(name);
     return 0;
 }
 
+int setenv(const char* name, const char* value, int overwrite)
+{
+    if (!overwrite && !getenv(name))
+        return 0;
+    auto length = strlen(name) + strlen(value) + 2;
+    auto* var = (char*)malloc(length);
+    snprintf(var, length, "%s=%s", name, value);
+    s_malloced_environment_variables.set(var);
+    return putenv(var);
+}
+
 int putenv(char* new_var)
 {
     char* new_eq = strchr(new_var, '=');
@@ -111,6 +134,7 @@ int putenv(char* new_var)
             continue; // can't match
 
         if (strncmp(new_var, old_var, new_var_len) == 0) {
+            free_environment_variable_if_needed(old_var);
             environ[environ_size] = new_var;
             return 0;
         }
diff --git a/LibC/stdlib.h b/LibC/stdlib.h
index 946a1d82748..a310ee5e11e 100644
--- a/LibC/stdlib.h
+++ b/LibC/stdlib.h
@@ -18,6 +18,7 @@ void* realloc(void* ptr, size_t);
 char* getenv(const char* name);
 int putenv(char*);
 int unsetenv(const char*);
+int setenv(const char* name, const char* value, int overwrite);
 int atoi(const char*);
 long atol(const char*);
 long long atoll(const char*);
diff --git a/Shell/main.cpp b/Shell/main.cpp
index b596fc2c931..47a2bc2ef92 100644
--- a/Shell/main.cpp
+++ b/Shell/main.cpp
@@ -72,13 +72,7 @@ static int sh_export(int argc, char** argv)
         return 1;
     }
 
-    // FIXME: Yes, this leaks.
-    // Maybe LibCore should grow a CEnvironment which is secretly a map to char*,
-    // so it can keep track of the environment pointers as needed?
-    const auto& s = String::format("%s=%s", parts[0].characters(), parts[1].characters());
-    char *ev = strndup(s.characters(), s.length());
-    putenv(ev);
-    return 0;
+    return setenv(parts[0].characters(), parts[1].characters(), 1);
 }
 
 static int sh_unset(int argc, char** argv)
@@ -494,9 +488,7 @@ int main(int argc, char** argv)
         if (pw) {
             g.username = pw->pw_name;
             g.home = pw->pw_dir;
-            const auto& s = String::format("HOME=%s", pw->pw_dir);
-            char *ev = strndup(s.characters(), s.length());
-            putenv(ev);
+            setenv("HOME", pw->pw_dir, 1);
         }
         endpwent();
     }