C++ify gdb/common/environ.c

As part of the preparation necessary for my upcoming task, I'd like to
propose that we turn gdb_environ into a class.  The approach taken
here is simple: the class gdb_environ contains everything that is
needed to manipulate the environment variables.  These variables are
stored in an std::vector<char *>, which can be converted to a 'char
**' and passed as argument to functions that need it.

The usage has not changed much.  As per Pedro's suggestion, this class
uses a static factory method initialization.  This means that when an
instance is created, it is initially empty.  When needed, it has to be
initialized using the static method 'from_host_environ'.

As mentioned before, this is a preparation for an upcoming work that I
will be posting in the next few weeks or so.  For that work, I'll
probably create another data structure that will contain all the
environment variables that were set by the user using the 'set
environment' command, because I'll need access to them.  This will be
much easier with the class-ification of gdb_environ.

As noted, this has been regression-tested with the new version of
environ.exp and no regressions were found.

gdb/ChangeLog:
2017-06-20  Sergio Durigan Junior  <sergiodj@redhat.com>

	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
	'unittests/environ-selftests.c'.
	(SUBDIR_UNITTESTS_OBS): Add 'environ-selftests.o'.
	* charset.c (find_charset_names): Declare object 'iconv_env'.
	Update code to use 'iconv_env' object.  Remove call to
	'free_environ'.
	* common/environ.c: Include <utility>.
	(make_environ): Delete function.
	(free_environ): Delete function.
	(gdb_environ::clear): New function.
	(gdb_environ::operator=): New function.
	(gdb_environ::get): Likewise.
	(environ_vector): Delete function.
	(set_in_environ): Delete function.
	(gdb_environ::set): New function.
	(unset_in_environ): Delete function.
	(gdb_environ::unset): New function.
	(gdb_environ::envp): Likewise.
	* common/environ.h: Include <vector>.
	(struct gdb_environ): Delete; transform into...
	(class gdb_environ): ... this class.
	(free_environ): Delete prototype.
	(init_environ, get_in_environ, set_in_environ, unset_in_environ,
	environ_vector): Likewise.
	* infcmd.c (run_command_1): Update code to call
	'envp' from 'gdb_environ' class.
	(environment_info): Update code to call methods from 'gdb_environ'
	class.
	(unset_environment_command): Likewise.
	(path_info): Likewise.
	(path_command): Likewise.
	* inferior.c (inferior::~inferior): Delete call to 'free_environ'.
	(inferior::inferior): Initialize 'environment' using the host's
	information.
	* inferior.h: Remove forward declaration of 'struct gdb_environ'.
	Include "environ.h".
	(class inferior) <environment>: Change type from 'struct
	gdb_environ' to 'gdb_environ'.
	* mi/mi-cmd-env.c (mi_cmd_env_path): Update code to call
	methods from 'gdb_environ' class.
	* solib.c (solib_find_1): Likewise
	* unittests/environ-selftests.c: New file.

gdb/gdbserver/ChangeLog:
2017-06-20  Sergio Durigan Junior  <sergiodj@redhat.com>

	* linux-low.c (linux_create_inferior): Adjust code to access the
	environment information via 'gdb_environ' class.
	* lynx-low.c (lynx_create_inferior): Likewise.
	* server.c (our_environ): Make it an instance of 'gdb_environ'.
	(get_environ): Return a pointer to 'our_environ'.
	(captured_main): Initialize 'our_environ'.
	* server.h (get_environ): Adjust prototype.
	* spu-low.c (spu_create_inferior): Adjust code to access the
	environment information via 'gdb_environ' class.

1 files changed, 151 insertions, 0 deletions

diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
new file mode 100644
index 0000000..ecc3955
--- /dev/null
+++ b/gdb/unittests/environ-selftests.c
@@ -0,0 +1,151 @@
+/* Self tests for gdb_environ for GDB, the GNU debugger.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "selftest.h"
+#include "common/environ.h"
+
+namespace selftests {
+namespace gdb_environ_tests {
+
+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;
+
+  /* When the vector is initialized, there should always be one NULL
+     element in it.  */
+  SELF_CHECK (env.envp ()[0] == NULL);
+
+  /* Make sure that there is no other element.  */
+  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);
+
+  /* Initialize the environment vector using the host's environ.  */
+  env = gdb_environ::from_host_environ ();
+
+  /* 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);
+
+  /* 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);
+
+  /* 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 ();
+  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.  */
+  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.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.clear ();
+
+  /* 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);
+  gdb_environ env2;
+  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);
+
+  /* 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);
+  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);
+
+  /* Test self-move.  */
+  env.clear ();
+  env.set ("A", "1");
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  env = std::move (env);
+  SELF_CHECK (strcmp (env.get ("A"), "1") == 0);
+  SELF_CHECK (strcmp (env.envp ()[0], "A=1") == 0);
+  SELF_CHECK (env.envp ()[1] == NULL);
+}
+} /* namespace gdb_environ */
+} /* namespace selftests */
+
+void
+_initialize_environ_selftests ()
+{
+  register_self_test (selftests::gdb_environ_tests::run_tests);
+}