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.

2 files changed, 129 insertions, 143 deletions

diff --git a/gdb/common/environ.c b/gdb/common/environ.c
index 3145d01..20f8aba 100644
--- a/gdb/common/environ.c
+++ b/gdb/common/environ.c
@@ -18,165 +18,119 @@
 #include "common-defs.h"
 #include "environ.h"
 #include <algorithm>
-
+#include <utility>
 
-/* Return a new environment object.  */
+/* See common/environ.h.  */
 
-struct gdb_environ *
-make_environ (void)
+gdb_environ &
+gdb_environ::operator= (gdb_environ &&e)
 {
-  struct gdb_environ *e;
-
-  e = XNEW (struct gdb_environ);
-
-  e->allocated = 10;
-  e->vector = (char **) xmalloc ((e->allocated + 1) * sizeof (char *));
-  e->vector[0] = 0;
-  return e;
+  /* Are we self-moving?  */
+  if (&e == this)
+    return *this;
+
+  m_environ_vector = std::move (e.m_environ_vector);
+  e.m_environ_vector.clear ();
+  e.m_environ_vector.push_back (NULL);
+  return *this;
 }
 
-/* Free an environment and all the strings in it.  */
+/* See common/environ.h.  */
 
-void
-free_environ (struct gdb_environ *e)
-{
-  char **vector = e->vector;
-
-  while (*vector)
-    xfree (*vector++);
-
-  xfree (e->vector);
-  xfree (e);
-}
-
-/* Copy the environment given to this process into E.
-   Also copies all the strings in it, so we can be sure
-   that all strings in these environments are safe to free.  */
-
-void
-init_environ (struct gdb_environ *e)
+gdb_environ gdb_environ::from_host_environ ()
 {
   extern char **environ;
-  int i;
+  gdb_environ e;
 
   if (environ == NULL)
-    return;
-
-  for (i = 0; environ[i]; i++) /*EMPTY */ ;
+    return e;
 
-  if (e->allocated < i)
+  for (int i = 0; environ[i] != NULL; ++i)
     {
-      e->allocated = std::max (i, e->allocated + 10);
-      e->vector = (char **) xrealloc ((char *) e->vector,
-				      (e->allocated + 1) * sizeof (char *));
+      /* Make sure we add the element before the last (NULL).  */
+      e.m_environ_vector.insert (e.m_environ_vector.end () - 1,
+				 xstrdup (environ[i]));
     }
 
-  memcpy (e->vector, environ, (i + 1) * sizeof (char *));
+  return e;
+}
 
-  while (--i >= 0)
-    {
-      int len = strlen (e->vector[i]);
-      char *newobj = (char *) xmalloc (len + 1);
+/* See common/environ.h.  */
 
-      memcpy (newobj, e->vector[i], len + 1);
-      e->vector[i] = newobj;
-    }
+void
+gdb_environ::clear ()
+{
+  for (char *v : m_environ_vector)
+    xfree (v);
+  m_environ_vector.clear ();
+  /* Always add the NULL element.  */
+  m_environ_vector.push_back (NULL);
 }
 
-/* Return the vector of environment E.
-   This is used to get something to pass to execve.  */
+/* Helper function to check if STRING contains an environment variable
+   assignment of VAR, i.e., if STRING starts with 'VAR='.  Return true
+   if it contains, false otherwise.  */
 
-char **
-environ_vector (struct gdb_environ *e)
+static bool
+match_var_in_string (char *string, const char *var, size_t var_len)
 {
-  return e->vector;
+  if (strncmp (string, var, var_len) == 0 && string[var_len] == '=')
+    return true;
+
+  return false;
 }
-
-/* Return the value in environment E of variable VAR.  */
 
-char *
-get_in_environ (const struct gdb_environ *e, const char *var)
+/* See common/environ.h.  */
+
+const char *
+gdb_environ::get (const char *var) const
 {
-  int len = strlen (var);
-  char **vector = e->vector;
-  char *s;
+  size_t len = strlen (var);
 
-  for (; (s = *vector) != NULL; vector++)
-    if (strncmp (s, var, len) == 0 && s[len] == '=')
-      return &s[len + 1];
+  for (char *el : m_environ_vector)
+    if (el != NULL && match_var_in_string (el, var, len))
+      return &el[len + 1];
 
-  return 0;
+  return NULL;
 }
 
-/* Store the value in E of VAR as VALUE.  */
+/* See common/environ.h.  */
 
 void
-set_in_environ (struct gdb_environ *e, const char *var, const char *value)
+gdb_environ::set (const char *var, const char *value)
 {
-  int i;
-  int len = strlen (var);
-  char **vector = e->vector;
-  char *s;
+  /* We have to unset the variable in the vector if it exists.  */
+  unset (var);
 
-  for (i = 0; (s = vector[i]) != NULL; i++)
-    if (strncmp (s, var, len) == 0 && s[len] == '=')
-      break;
-
-  if (s == 0)
-    {
-      if (i == e->allocated)
-	{
-	  e->allocated += 10;
-	  vector = (char **) xrealloc ((char *) vector,
-				       (e->allocated + 1) * sizeof (char *));
-	  e->vector = vector;
-	}
-      vector[i + 1] = 0;
-    }
-  else
-    xfree (s);
-
-  s = (char *) xmalloc (len + strlen (value) + 2);
-  strcpy (s, var);
-  strcat (s, "=");
-  strcat (s, value);
-  vector[i] = s;
-
-  /* This used to handle setting the PATH and GNUTARGET variables
-     specially.  The latter has been replaced by "set gnutarget"
-     (which has worked since GDB 4.11).  The former affects searching
-     the PATH to find SHELL, and searching the PATH to find the
-     argument of "symbol-file" or "exec-file".  Maybe we should have
-     some kind of "set exec-path" for that.  But in any event, having
-     "set env" affect anything besides the inferior is a bad idea.
-     What if we want to change the environment we pass to the program
-     without afecting GDB's behavior?  */
-
-  return;
+  /* Insert the element before the last one, which is always NULL.  */
+  m_environ_vector.insert (m_environ_vector.end () - 1,
+			   concat (var, "=", value, NULL));
 }
 
-/* Remove the setting for variable VAR from environment E.  */
+/* See common/environ.h.  */
 
 void
-unset_in_environ (struct gdb_environ *e, const char *var)
+gdb_environ::unset (const char *var)
 {
-  int len = strlen (var);
-  char **vector = e->vector;
-  char *s;
+  size_t len = strlen (var);
+
+  /* We iterate until '.cend () - 1' because the last element is
+     always NULL.  */
+  for (std::vector<char *>::const_iterator el = m_environ_vector.cbegin ();
+       el != m_environ_vector.cend () - 1;
+       ++el)
+    if (match_var_in_string (*el, var, len))
+      {
+	xfree (*el);
+	m_environ_vector.erase (el);
+	break;
+      }
+}
 
-  for (; (s = *vector) != NULL; vector++)
-    {
-      if (strncmp (s, var, len) == 0 && s[len] == '=')
-	{
-	  xfree (s);
-	  /* Walk through the vector, shuffling args down by one, including
-	     the NULL terminator.  Can't use memcpy() here since the regions
-	     overlap, and memmove() might not be available.  */
-	  while ((vector[0] = vector[1]) != NULL)
-	    {
-	      vector++;
-	    }
-	  break;
-	}
-    }
+/* See common/environ.h.  */
+
+char **
+gdb_environ::envp () const
+{
+  return const_cast<char **> (&m_environ_vector[0]);
 }
diff --git a/gdb/common/environ.h b/gdb/common/environ.h
index 3ace69e..0bbb191 100644
--- a/gdb/common/environ.h
+++ b/gdb/common/environ.h
@@ -17,33 +17,65 @@
 #if !defined (ENVIRON_H)
 #define ENVIRON_H 1
 
-/* We manipulate environments represented as these structures.  */
+#include <vector>
 
-struct gdb_environ
+/* Class that represents the environment variables as seen by the
+   inferior.  */
+
+class gdb_environ
+{
+public:
+  /* Regular constructor and destructor.  */
+  gdb_environ ()
+  {
+    /* Make sure that the vector contains at least a NULL element.
+       If/when we add more variables to it, NULL will always be the
+       last element.  */
+    m_environ_vector.push_back (NULL);
+  }
+
+  ~gdb_environ ()
   {
-    /* Number of usable slots allocated in VECTOR.
-       VECTOR always has one slot not counted here,
-       to hold the terminating zero.  */
-    int allocated;
-    /* A vector of slots, ALLOCATED + 1 of them.
-       The first few slots contain strings "VAR=VALUE"
-       and the next one contains zero.
-       Then come some unused slots.  */
-    char **vector;
-  };
+    clear ();
+  }
+
+  /* Move constructor.  */
+  gdb_environ (gdb_environ &&e)
+    : m_environ_vector (std::move (e.m_environ_vector))
+  {
+    /* Make sure that the moved-from vector is left at a valid
+       state (only one NULL element).  */
+    e.m_environ_vector.clear ();
+    e.m_environ_vector.push_back (NULL);
+  }
+
+  /* Move assignment.  */
+  gdb_environ &operator= (gdb_environ &&e);
 
-extern struct gdb_environ *make_environ (void);
+  /* Create a gdb_environ object using the host's environment
+     variables.  */
+  static gdb_environ from_host_environ ();
 
-extern void free_environ (struct gdb_environ *);
+  /* Clear the environment variables stored in the object.  */
+  void clear ();
 
-extern void init_environ (struct gdb_environ *);
+  /* Return the value in the environment for the variable VAR.  The
+     returned pointer is only valid as long as the gdb_environ object
+     is not modified.  */
+  const char *get (const char *var) const;
 
-extern char *get_in_environ (const struct gdb_environ *, const char *);
+  /* Store VAR=VALUE in the environment.  */
+  void set (const char *var, const char *value);
 
-extern void set_in_environ (struct gdb_environ *, const char *, const char *);
+  /* Unset VAR in environment.  */
+  void unset (const char *var);
 
-extern void unset_in_environ (struct gdb_environ *, const char *);
+  /* Return the environment vector represented as a 'char **'.  */
+  char **envp () const;
 
-extern char **environ_vector (struct gdb_environ *);
+private:
+  /* A vector containing the environment variables.  */
+  std::vector<char *> m_environ_vector;
+};
 
 #endif /* defined (ENVIRON_H) */