Add a second test for environment functionality

src/includes/dinit-env.h

@@ -16,8 +16,9 @@ extern environment main_env;
 // May throw bad_alloc or system_error.
 void read_env_file(const char *file, bool log_warnings, environment &env);
 
-// Note that our sets (defined as part of environment class below) allow searching based
-// on a name only (string_view) or "NAME=VALUE" assignment pair (std::string).
+// Note that our sets (defined as part of environment class below) allow searching based on a name
+// only (string_view) or "NAME=VALUE" assignment pair (std::string). It is important to always
+// search using the correct type.
 
 // Hash environment variable name only (not including value)
 struct hash_env_name
@@ -34,6 +35,7 @@ struct hash_env_name
     }
 };
 
+// Comparison predicate for environment variables, checking name only
 struct env_equal_name
 {
     bool operator()(const std::string &a, const std::string &b) const noexcept
@@ -88,7 +90,7 @@ class environment
 public:
 
     struct env_map {
-        // list of environment variables, i.e. list as suitable for exec
+        // *non-owning* list of environment variables, i.e. list as suitable for exec
         std::vector<const char *> env_list;
 
         // map of variable name (via string_view) to its index in env_list
@@ -116,7 +118,9 @@ public:
         return {name_and_val, name.length() + 1 + val_len};
     }
 
-    // build a mapping excluding named variables (only called if the parent is the real environment)
+    // Build a mapping excluding named variables (only called if the parent is the real environment).
+    // Note that the return is non-owning, i.e. the variable values are backed by the environment object
+    // and their lifetime is bounded to it.
     env_map build(const env_names &exclude) const {
         env_map mapping;
 

src/tests/envtests.cc

@@ -1,5 +1,7 @@
 #include <iostream>
+#include <string>
 #include <cassert>
+#include <cstring>
 
 #include "baseproc-sys.h"
 
@@ -28,6 +30,68 @@ void test_env_1()
     bp_sys::clearenv();
 }
 
+void test_env_2()
+{
+    using namespace bp_sys;
+
+    bp_sys::clearenv();
+
+    environment env1; // parent
+    environment env2;
+
+    bp_sys::setenv("VAR1","VAR1-env",1); // value to be inherited right through
+    bp_sys::setenv("VAR2","VAR2-env",1); // value to be overridden in env1
+    bp_sys::setenv("VAR3","VAR3-env",1); // value to be undefined in env1
+    bp_sys::setenv("VAR4","VAR4-env",1); // value to be undefined in env1 and overridden in env2
+    bp_sys::setenv("VAR5","VAR5-env",1); // value to be overridden in env2
+    bp_sys::setenv("VAR6","VAR6-env",1); // value to be overridden in both env1 and env2
+    bp_sys::setenv("VAR7","VAR7-env",1); // value to be overridden env1 and undefined in env2
+    bp_sys::setenv("VAR8","VAR8-env",1); // value to be undefined in env2
+
+    env1.set_var("VAR2=VAR2-env1");
+    env1.undefine_var("VAR3");
+    env1.undefine_var("VAR4");
+    env1.set_var("VAR6=VAR6-env1");
+    env1.set_var("VAR7=VAR7-env1");
+
+    env2.set_var("VAR4=VAR4-env2");
+    env2.set_var("VAR5=VAR5-env2");
+    env2.set_var("VAR6=VAR6-env2");
+    env2.undefine_var("VAR7");
+    env2.undefine_var("VAR8");
+
+    environment::env_map mapping = env2.build(env1);
+
+    assert(mapping.env_list.size() == 6); // 5 + null terminator
+
+    auto checkvalue = [&](const std::string &name, const char *val) {
+        for (const char *vv : mapping.env_list) {
+            if (vv == nullptr) break;
+            if (strncmp(vv, name.c_str(), name.size()) == 0 && vv[name.size()] == '=') {
+                const char *vv_val = vv + name.size() + 1;
+                assert(val != nullptr && "expected undefined variable, but has value");
+                if (strcmp(vv_val, val) != 0) {
+                    std::cout << "\nfor: " << name << ", expected: " << val << ", actual: " << vv_val << "\n";
+                }
+                assert(strcmp(vv_val, val) == 0 && "variable has wrong value");
+                return;
+            }
+        }
+        assert(val == nullptr && "expected defined variable, but variable not found");
+    };
+
+    checkvalue("VAR1", "VAR1-env");
+    checkvalue("VAR2", "VAR2-env1");
+    checkvalue("VAR3", nullptr);
+    checkvalue("VAR4", "VAR4-env2");
+    checkvalue("VAR5", "VAR5-env2");
+    checkvalue("VAR6", "VAR6-env2");
+    checkvalue("VAR7", nullptr);
+    checkvalue("VAR8", nullptr);
+
+    bp_sys::clearenv();
+}
+
 #define RUN_TEST(name, spacing) \
     std::cout << #name "..." spacing << std::flush; \
     name(); \
@@ -36,4 +100,5 @@ void test_env_1()
 int main(int argc, char **argv)
 {
     RUN_TEST(test_env_1, "                ");
+    RUN_TEST(test_env_2, "                ");
 }