Re: [PATCH v4] Implement the ability to set/unset environment variables to GDBserver when starting the inferior

[Simon Marchi <simon dot marchi at ericsson dot com>]

> diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
> index 1e938e6746..e2d8c70985 100644
> --- a/gdb/unittests/environ-selftests.c
> +++ b/gdb/unittests/environ-selftests.c
> @@ -22,132 +22,257 @@
>  #include "common/environ.h"
>  #include "common/diagnostics.h"
>  
> +static bool
> +set_contains (const std::set<std::string> &set, std::string key)
> +{
> +  return set.find (key) != set.end ();
> +}
> +
>  namespace selftests {
>  namespace gdb_environ_tests {
>  
> +/* Test if the vector is initialized in a correct way.  */
> +
>  static void
> -run_tests ()
> +test_vector_initialization (gdb_environ *env)
>  {
> -  /* Set a test environment variable.  This will be unset at the end
> -     of this function.  */
> -  if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0)
> -    error (_("Could not set environment variable for testing."));
> -
> -  gdb_environ env;
> -
>    /* When the vector is initialized, there should always be one NULL
>       element in it.  */
> -  SELF_CHECK (env.envp ()[0] == NULL);
> +  SELF_CHECK (env->envp ()[0] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 0);
> +  SELF_CHECK (env->user_unset_env ().size () == 0);
>  
>    /* Make sure that there is no other element.  */
> -  SELF_CHECK (env.get ("PWD") == NULL);
> +  SELF_CHECK (env->get ("PWD") == NULL);
> +}
>  
> -  /* Check if unset followed by a set in an empty vector works.  */
> -  env.set ("PWD", "test");
> -  SELF_CHECK (strcmp (env.get ("PWD"), "test") == 0);
> -  /* The second element must be NULL.  */
> -  SELF_CHECK (env.envp ()[1] == NULL);
> -  env.unset ("PWD");
> -  SELF_CHECK (env.envp ()[0] == NULL);
> +/* Test initialization using the host's environ.  */
>  
> +static void
> +test_init_from_host_environ (gdb_environ *env)
> +{
>    /* Initialize the environment vector using the host's environ.  */
> -  env = gdb_environ::from_host_environ ();
> +  *env = gdb_environ::from_host_environ ();
> +
> +  /* The user-set and user-unset lists must be empty.  */
> +  SELF_CHECK (env->user_set_env ().size () == 0);
> +  SELF_CHECK (env->user_unset_env ().size () == 0);
>  
>    /* Our test environment variable should be present at the
>       vector.  */
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
> -
> -  /* Set our test variable to another value.  */
> -  env.set ("GDB_SELFTEST_ENVIRON", "test");
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "test") == 0);
> -
> -  /* And unset our test variable.  The variable still exists in the
> -     host's environment, but doesn't exist in our vector.  */
> -  env.unset ("GDB_SELFTEST_ENVIRON");
> -  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL);
> -
> -  /* Re-set the test variable.  */
> -  env.set ("GDB_SELFTEST_ENVIRON", "1");
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
> +}
>  
> -  /* When we clear our environ vector, there should be only one
> -     element on it (NULL), and we shouldn't be able to get our test
> -     variable.  */
> -  env.clear ();
> -  SELF_CHECK (env.envp ()[0] == NULL);
> -  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL);
> +/* Test reinitialization using the host's environ.  */
>  
> +static void
> +test_reinit_from_host_environ (gdb_environ *env)
> +{
>    /* Reinitialize our environ vector using the host environ.  We
>       should be able to see one (and only one) instance of the test
>       variable.  */
> -  env = gdb_environ::from_host_environ ();
> -  char **envp = env.envp ();
> +  *env = gdb_environ::from_host_environ ();
> +  char **envp = env->envp ();
>    int num_found = 0;
>  
>    for (size_t i = 0; envp[i] != NULL; ++i)
>      if (strcmp (envp[i], "GDB_SELFTEST_ENVIRON=1") == 0)
>        ++num_found;
>    SELF_CHECK (num_found == 1);
> +}
>  
> -  /* Get rid of our test variable.  */
> -  unsetenv ("GDB_SELFTEST_ENVIRON");
> +/* Test the case when we set a variable A, then set a variable B,
> +   then unset A, and make sure that we cannot find A in the environ
> +   vector, but can still find B.  */
> +
> +static void
> +test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (gdb_environ *env)
> +{
> +  env->set ("GDB_SELFTEST_ENVIRON_1", "aaa");
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0);
> +  /* User-set environ var list must contain one element.  */
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +  SELF_CHECK (set_contains (env->user_set_env (),
> +			    std::string ("GDB_SELFTEST_ENVIRON_1=aaa")));
>  
> -  /* Test the case when we set a variable A, then set a variable B,
> -     then unset A, and make sure that we cannot find A in the environ
> -     vector, but can still find B.  */
> -  env.set ("GDB_SELFTEST_ENVIRON_1", "aaa");
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0);
> +  env->set ("GDB_SELFTEST_ENVIRON_2", "bbb");
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
>  
> -  env.set ("GDB_SELFTEST_ENVIRON_2", "bbb");
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
> +  env->unset ("GDB_SELFTEST_ENVIRON_1");
> +  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON_1") == NULL);
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
>  
> -  env.unset ("GDB_SELFTEST_ENVIRON_1");
> -  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == NULL);
> -  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
> +  /* The user-set environ var list must contain only one element
> +     now.  */
> +  SELF_CHECK (set_contains (env->user_set_env (),
> +			    std::string ("GDB_SELFTEST_ENVIRON_2=bbb")));
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +}
>  
> -  env.clear ();
> +/* Check if unset followed by a set in an empty vector works.  */
>  
> -  /* Test that after a std::move the moved-from object is left at a
> -     valid state (i.e., its only element is NULL).  */
> -  env.set ("A", "1");
> -  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
> +static void
> +test_unset_set_empty_vector (gdb_environ *env)
> +{
> +  env->set ("PWD", "test");
> +  SELF_CHECK (strcmp (env->get ("PWD"), "test") == 0);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("PWD=test")));
> +  SELF_CHECK (env->user_unset_env ().size () == 0);
> +  /* The second element must be NULL.  */
> +  SELF_CHECK (env->envp ()[1] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +  env->unset ("PWD");
> +  SELF_CHECK (env->envp ()[0] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 0);
> +  SELF_CHECK (env->user_unset_env ().size () == 1);
> +  SELF_CHECK (set_contains (env->user_unset_env (), std::string ("PWD")));
> +}
> +
> +/* When we clear our environ vector, there should be only one
> +   element on it (NULL), and we shouldn't be able to get our test
> +   variable.  */
> +
> +static void
> +test_vector_clear (gdb_environ *env)
> +{
> +  env->clear ();
> +  SELF_CHECK (env->envp ()[0] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 0);
> +  SELF_CHECK (env->user_unset_env ().size () == 0);
> +  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL);
> +}
> +
> +/* Test that after a std::move the moved-from object is left at a
> +   valid state (i.e., its only element is NULL).  */
> +
> +static void
> +test_std_move (gdb_environ *env)
> +{
> +  env->set ("A", "1");
> +  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
> +  SELF_CHECK (env->user_set_env ().size () == 1);
>    gdb_environ env2;
> -  env2 = std::move (env);
> -  SELF_CHECK (env.envp ()[0] == NULL);
> +  env2 = std::move (*env);
> +  SELF_CHECK (env->envp ()[0] == NULL);
>    SELF_CHECK (strcmp (env2.get ("A"), "1") == 0);
>    SELF_CHECK (env2.envp ()[1] == NULL);
> -  env.set ("B", "2");
> -  SELF_CHECK (strcmp (env.get ("B"), "2") == 0);
> -  SELF_CHECK (env.envp ()[1] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 0);
> +  SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1")));
> +  SELF_CHECK (env2.user_set_env ().size () == 1);
> +  env->set ("B", "2");
> +  SELF_CHECK (strcmp (env->get ("B"), "2") == 0);
> +  SELF_CHECK (env->envp ()[1] == NULL);
> +}
>  
> -  /* Test that the move constructor leaves everything at a valid
> -     state.  */
> -  env.clear ();
> -  env.set ("A", "1");
> -  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
> -  gdb_environ env3 = std::move (env);
> -  SELF_CHECK (env.envp ()[0] == NULL);
> +/* Test that the move constructor leaves everything at a valid
> +   state.  */
> +
> +static void
> +test_move_constructor (gdb_environ *env)
> +{
> +  env->clear ();
> +  env->set ("A", "1");
> +  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
> +  gdb_environ env3 = std::move (*env);
> +  SELF_CHECK (env->envp ()[0] == NULL);
> +  SELF_CHECK (env->user_set_env ().size () == 0);
>    SELF_CHECK (strcmp (env3.get ("A"), "1") == 0);
>    SELF_CHECK (env3.envp ()[1] == NULL);
> -  env.set ("B", "2");
> -  SELF_CHECK (strcmp (env.get ("B"), "2") == 0);
> -  SELF_CHECK (env.envp ()[1] == NULL);
> +  SELF_CHECK (set_contains (env3.user_set_env (), std::string ("A=1")));
> +  SELF_CHECK (env3.user_set_env ().size () == 1);
> +  env->set ("B", "2");
> +  SELF_CHECK (strcmp (env->get ("B"), "2") == 0);
> +  SELF_CHECK (env->envp ()[1] == NULL);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("B=2")));
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +}
> +
> +/* Test that self-moving works.  */
>  
> +static void
> +test_self_move (gdb_environ *env)
> +{
>    /* Test self-move.  */
> -  env.clear ();
> -  env.set ("A", "1");
> -  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
> +  env->clear ();
> +  env->set ("A", "1");
> +  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
> +  SELF_CHECK (env->user_set_env ().size () == 1);
>  
>    /* Some compilers warn about moving to self, but that's precisely what we want
>       to test here, so turn this warning off.  */
>    DIAGNOSTIC_PUSH
>    DIAGNOSTIC_IGNORE_SELF_MOVE
> -  env = std::move (env);
> +  *env = std::move (*env);
>    DIAGNOSTIC_POP
>  
> -  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
> -  SELF_CHECK (strcmp (env.envp ()[0], "A=1") == 0);
> -  SELF_CHECK (env.envp ()[1] == NULL);
> +  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
> +  SELF_CHECK (strcmp (env->envp ()[0], "A=1") == 0);
> +  SELF_CHECK (env->envp ()[1] == NULL);
> +  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +}
> +
> +/* Test if set/unset/reset works.  */
> +
> +static void
> +test_set_unset_reset (gdb_environ *env)
> +{
> +  /* Set our test variable to another value.  */
> +  env->set ("GDB_SELFTEST_ENVIRON", "test");
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "test") == 0);
> +  SELF_CHECK (env->user_set_env ().size () == 1);
> +  SELF_CHECK (env->user_unset_env ().size () == 0);
> +
> +  /* And unset our test variable.  The variable still exists in the
> +     host's environment, but doesn't exist in our vector.  */
> +  env->unset ("GDB_SELFTEST_ENVIRON");
> +  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL);
> +  SELF_CHECK (env->user_unset_env ().size () == 1);
> +  SELF_CHECK (set_contains (env->user_unset_env (),
> +			    std::string ("GDB_SELFTEST_ENVIRON")));
> +
> +  /* Re-set the test variable.  */
> +  env->set ("GDB_SELFTEST_ENVIRON", "1");
> +  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
> +}
> +
> +static void
> +run_tests ()
> +{
> +  /* Set a test environment variable.  This will be unset at the end
> +     of this function.  */
> +  if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0)
> +    error (_("Could not set environment variable for testing."));
> +
> +  gdb_environ env;
> +
> +  test_vector_initialization (&env);
> +
> +  test_unset_set_empty_vector (&env);
> +
> +  test_init_from_host_environ (&env);
> +
> +  test_set_unset_reset (&env);
> +
> +  test_vector_clear (&env);
> +
> +  test_reinit_from_host_environ (&env);
> +
> +  /* Get rid of our test variable.  We won't need it anymore.  */
> +  unsetenv ("GDB_SELFTEST_ENVIRON");
> +
> +  test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (&env);
> +
> +  env.clear ();
> +
> +  test_std_move (&env);
> +
> +  test_move_constructor (&env);
> +
> +  test_self_move (&env);
>  }
>  } /* namespace gdb_environ */
>  } /* namespace selftests */
> 

Hi Sergio,

The main point of splitting that long test into separate functions is to have each
of them independent from each other.  So they should ideally not use the same environment
object.  I suggest doing something like this (applied on top of your patch).  I did this
super quickly, so please check that the tests still make sense :)


>From 0a7d0381844d640792cc3f3051f643db8df42937 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 29 Aug 2017 17:06:51 +0200
Subject: [PATCH] stuff

---
 gdb/unittests/environ-selftests.c | 238 +++++++++++++++++++++-----------------
 1 file changed, 130 insertions(+), 108 deletions(-)

diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index e2d8c70..72a46af 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -22,6 +22,8 @@
 #include "common/environ.h"
 #include "common/diagnostics.h"

+static const char *gdb_selftest_env_var = "GDB_SELFTEST_ENVIRON";
+
 static bool
 set_contains (const std::set<std::string> &set, std::string key)
 {
@@ -34,45 +36,48 @@ namespace gdb_environ_tests {
 /* Test if the vector is initialized in a correct way.  */

 static void
-test_vector_initialization (gdb_environ *env)
+test_vector_initialization ()
 {
+  gdb_environ env;
+
   /* When the vector is initialized, there should always be one NULL
      element in it.  */
-  SELF_CHECK (env->envp ()[0] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 0);
-  SELF_CHECK (env->user_unset_env ().size () == 0);
+  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (env.user_unset_env ().size () == 0);

   /* Make sure that there is no other element.  */
-  SELF_CHECK (env->get ("PWD") == NULL);
+  SELF_CHECK (env.get ("PWD") == NULL);
 }

 /* Test initialization using the host's environ.  */

 static void
-test_init_from_host_environ (gdb_environ *env)
+test_init_from_host_environ ()
 {
   /* Initialize the environment vector using the host's environ.  */
-  *env = gdb_environ::from_host_environ ();
+  gdb_environ env = gdb_environ::from_host_environ ();

   /* The user-set and user-unset lists must be empty.  */
-  SELF_CHECK (env->user_set_env ().size () == 0);
-  SELF_CHECK (env->user_unset_env ().size () == 0);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (env.user_unset_env ().size () == 0);

   /* Our test environment variable should be present at the
      vector.  */
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
+  SELF_CHECK (strcmp (env.get (gdb_selftest_env_var), "1") == 0);
 }

 /* Test reinitialization using the host's environ.  */

 static void
-test_reinit_from_host_environ (gdb_environ *env)
+test_reinit_from_host_environ ()
 {
   /* Reinitialize our environ vector using the host environ.  We
      should be able to see one (and only one) instance of the test
      variable.  */
-  *env = gdb_environ::from_host_environ ();
-  char **envp = env->envp ();
+  gdb_environ env = gdb_environ::from_host_environ ();
+  env = gdb_environ::from_host_environ ();
+  char **envp = env.envp ();
   int num_found = 0;

   for (size_t i = 0; envp[i] != NULL; ++i)
@@ -86,46 +91,50 @@ test_reinit_from_host_environ (gdb_environ *env)
    vector, but can still find B.  */

 static void
-test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (gdb_environ *env)
+test_set_A_unset_B_unset_A_cannot_find_A_can_find_B ()
 {
-  env->set ("GDB_SELFTEST_ENVIRON_1", "aaa");
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0);
+  gdb_environ env;
+
+  env.set ("GDB_SELFTEST_ENVIRON_1", "aaa");
+  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0);
   /* User-set environ var list must contain one element.  */
-  SELF_CHECK (env->user_set_env ().size () == 1);
-  SELF_CHECK (set_contains (env->user_set_env (),
+  SELF_CHECK (env.user_set_env ().size () == 1);
+  SELF_CHECK (set_contains (env.user_set_env (),
 			    std::string ("GDB_SELFTEST_ENVIRON_1=aaa")));

-  env->set ("GDB_SELFTEST_ENVIRON_2", "bbb");
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
+  env.set ("GDB_SELFTEST_ENVIRON_2", "bbb");
+  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);

-  env->unset ("GDB_SELFTEST_ENVIRON_1");
-  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON_1") == NULL);
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);
+  env.unset ("GDB_SELFTEST_ENVIRON_1");
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == NULL);
+  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0);

   /* The user-set environ var list must contain only one element
      now.  */
-  SELF_CHECK (set_contains (env->user_set_env (),
+  SELF_CHECK (set_contains (env.user_set_env (),
 			    std::string ("GDB_SELFTEST_ENVIRON_2=bbb")));
-  SELF_CHECK (env->user_set_env ().size () == 1);
+  SELF_CHECK (env.user_set_env ().size () == 1);
 }

 /* Check if unset followed by a set in an empty vector works.  */

 static void
-test_unset_set_empty_vector (gdb_environ *env)
+test_unset_set_empty_vector ()
 {
-  env->set ("PWD", "test");
-  SELF_CHECK (strcmp (env->get ("PWD"), "test") == 0);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("PWD=test")));
-  SELF_CHECK (env->user_unset_env ().size () == 0);
+  gdb_environ env;
+
+  env.set ("PWD", "test");
+  SELF_CHECK (strcmp (env.get ("PWD"), "test") == 0);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("PWD=test")));
+  SELF_CHECK (env.user_unset_env ().size () == 0);
   /* The second element must be NULL.  */
-  SELF_CHECK (env->envp ()[1] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 1);
-  env->unset ("PWD");
-  SELF_CHECK (env->envp ()[0] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 0);
-  SELF_CHECK (env->user_unset_env ().size () == 1);
-  SELF_CHECK (set_contains (env->user_unset_env (), std::string ("PWD")));
+  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 1);
+  env.unset ("PWD");
+  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (env.user_unset_env ().size () == 1);
+  SELF_CHECK (set_contains (env.user_unset_env (), std::string ("PWD")));
 }

 /* When we clear our environ vector, there should be only one
@@ -133,110 +142,127 @@ test_unset_set_empty_vector (gdb_environ *env)
    variable.  */

 static void
-test_vector_clear (gdb_environ *env)
+test_vector_clear ()
 {
-  env->clear ();
-  SELF_CHECK (env->envp ()[0] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 0);
-  SELF_CHECK (env->user_unset_env ().size () == 0);
-  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL);
+  gdb_environ env;
+
+  env.set (gdb_selftest_env_var, "1");
+
+  env.clear ();
+  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (env.user_unset_env ().size () == 0);
+  SELF_CHECK (env.get (gdb_selftest_env_var) == NULL);
 }

 /* Test that after a std::move the moved-from object is left at a
    valid state (i.e., its only element is NULL).  */

 static void
-test_std_move (gdb_environ *env)
+test_std_move ()
 {
-  env->set ("A", "1");
-  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
-  SELF_CHECK (env->user_set_env ().size () == 1);
+  gdb_environ env;
   gdb_environ env2;
-  env2 = std::move (*env);
-  SELF_CHECK (env->envp ()[0] == NULL);
+
+  env.set ("A", "1");
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
+  SELF_CHECK (env.user_set_env ().size () == 1);
+
+  env2 = std::move (env);
+  SELF_CHECK (env.envp ()[0] == NULL);
   SELF_CHECK (strcmp (env2.get ("A"), "1") == 0);
   SELF_CHECK (env2.envp ()[1] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 0);
+  SELF_CHECK (env.user_set_env ().size () == 0);
   SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1")));
   SELF_CHECK (env2.user_set_env ().size () == 1);
-  env->set ("B", "2");
-  SELF_CHECK (strcmp (env->get ("B"), "2") == 0);
-  SELF_CHECK (env->envp ()[1] == NULL);
+  env.set ("B", "2");
+  SELF_CHECK (strcmp (env.get ("B"), "2") == 0);
+  SELF_CHECK (env.envp ()[1] == NULL);
 }

 /* Test that the move constructor leaves everything at a valid
    state.  */

 static void
-test_move_constructor (gdb_environ *env)
+test_move_constructor ()
 {
-  env->clear ();
-  env->set ("A", "1");
-  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
-  gdb_environ env3 = std::move (*env);
-  SELF_CHECK (env->envp ()[0] == NULL);
-  SELF_CHECK (env->user_set_env ().size () == 0);
-  SELF_CHECK (strcmp (env3.get ("A"), "1") == 0);
-  SELF_CHECK (env3.envp ()[1] == NULL);
-  SELF_CHECK (set_contains (env3.user_set_env (), std::string ("A=1")));
-  SELF_CHECK (env3.user_set_env ().size () == 1);
-  env->set ("B", "2");
-  SELF_CHECK (strcmp (env->get ("B"), "2") == 0);
-  SELF_CHECK (env->envp ()[1] == NULL);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("B=2")));
-  SELF_CHECK (env->user_set_env ().size () == 1);
+  gdb_environ env;
+
+  env.set ("A", "1");
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
+
+  gdb_environ env2 = std::move (env);
+  SELF_CHECK (env.envp ()[0] == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (strcmp (env2.get ("A"), "1") == 0);
+  SELF_CHECK (env2.envp ()[1] == NULL);
+  SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1")));
+  SELF_CHECK (env2.user_set_env ().size () == 1);
+
+  env.set ("B", "2");
+  SELF_CHECK (strcmp (env.get ("B"), "2") == 0);
+  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("B=2")));
+  SELF_CHECK (env.user_set_env ().size () == 1);
 }

 /* Test that self-moving works.  */

 static void
-test_self_move (gdb_environ *env)
+test_self_move ()
 {
+  gdb_environ env;
+
   /* Test self-move.  */
-  env->clear ();
-  env->set ("A", "1");
-  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
-  SELF_CHECK (env->user_set_env ().size () == 1);
+  env.set ("A", "1");
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
+  SELF_CHECK (env.user_set_env ().size () == 1);

   /* Some compilers warn about moving to self, but that's precisely what we want
      to test here, so turn this warning off.  */
   DIAGNOSTIC_PUSH
   DIAGNOSTIC_IGNORE_SELF_MOVE
-  *env = std::move (*env);
+  env = std::move (env);
   DIAGNOSTIC_POP

-  SELF_CHECK (strcmp (env->get ("A"), "1") == 0);
-  SELF_CHECK (strcmp (env->envp ()[0], "A=1") == 0);
-  SELF_CHECK (env->envp ()[1] == NULL);
-  SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1")));
-  SELF_CHECK (env->user_set_env ().size () == 1);
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  SELF_CHECK (strcmp (env.envp ()[0], "A=1") == 0);
+  SELF_CHECK (env.envp ()[1] == NULL);
+  SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1")));
+  SELF_CHECK (env.user_set_env ().size () == 1);
 }

 /* Test if set/unset/reset works.  */

 static void
-test_set_unset_reset (gdb_environ *env)
+test_set_unset_reset ()
 {
+  gdb_environ env = gdb_environ::from_host_environ ();
+
+  /* Our test variable should already be present in the host's environment.  */
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") != NULL);
+
   /* Set our test variable to another value.  */
-  env->set ("GDB_SELFTEST_ENVIRON", "test");
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "test") == 0);
-  SELF_CHECK (env->user_set_env ().size () == 1);
-  SELF_CHECK (env->user_unset_env ().size () == 0);
+  env.set ("GDB_SELFTEST_ENVIRON", "test");
+  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "test") == 0);
+  SELF_CHECK (env.user_set_env ().size () == 1);
+  SELF_CHECK (env.user_unset_env ().size () == 0);

   /* And unset our test variable.  The variable still exists in the
      host's environment, but doesn't exist in our vector.  */
-  env->unset ("GDB_SELFTEST_ENVIRON");
-  SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL);
-  SELF_CHECK (env->user_unset_env ().size () == 1);
-  SELF_CHECK (set_contains (env->user_unset_env (),
+  env.unset ("GDB_SELFTEST_ENVIRON");
+  SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL);
+  SELF_CHECK (env.user_set_env ().size () == 0);
+  SELF_CHECK (env.user_unset_env ().size () == 1);
+  SELF_CHECK (set_contains (env.user_unset_env (),
 			    std::string ("GDB_SELFTEST_ENVIRON")));

   /* Re-set the test variable.  */
-  env->set ("GDB_SELFTEST_ENVIRON", "1");
-  SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
+  env.set ("GDB_SELFTEST_ENVIRON", "1");
+  SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0);
 }

 static void
@@ -247,32 +273,28 @@ run_tests ()
   if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0)
     error (_("Could not set environment variable for testing."));

-  gdb_environ env;
+  test_vector_initialization ();

-  test_vector_initialization (&env);
+  test_unset_set_empty_vector ();

-  test_unset_set_empty_vector (&env);
+  test_init_from_host_environ ();

-  test_init_from_host_environ (&env);
+  test_set_unset_reset ();

-  test_set_unset_reset (&env);
+  test_vector_clear ();

-  test_vector_clear (&env);
-
-  test_reinit_from_host_environ (&env);
+  test_reinit_from_host_environ ();

   /* Get rid of our test variable.  We won't need it anymore.  */
   unsetenv ("GDB_SELFTEST_ENVIRON");

-  test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (&env);
-
-  env.clear ();
+  test_set_A_unset_B_unset_A_cannot_find_A_can_find_B ();

-  test_std_move (&env);
+  test_std_move ();

-  test_move_constructor (&env);
+  test_move_constructor ();

-  test_self_move (&env);
+  test_self_move ();
 }
 } /* namespace gdb_environ */
 } /* namespace selftests */
-- 
2.7.4