This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] C++ify gdb/common/environ.c
On 2017-04-17 23:03, Sergio Durigan Junior wrote:
Changes from v2 (mostly from Simon's review):
- Not using std::unique_ptr for gdb_environ anymore.
- Got rid of "_environ" suffixes from the names of the methods.
- Remove (void) from methods without parameters.
- Constify return of 'get' method.
- Change 'const char *' for 'const std::string &' on methods'
parameters.
- Returning a 'const std::vector<char *> &' on the 'get_vector'
method.
- Typos, and other minor nits.
Disclaimer: this patch depends on
<https://sourceware.org/ml/gdb-patches/2017-03/msg00551.html> to be
fully testable.
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 two data structures: an unordered_set, good because lookups
are O(n), and an std::vector<char *>, which can be converted to a
'char **' and passed as argument to functions that need it.
It still says O(n) ;)
diff --git a/gdb/common/environ.c b/gdb/common/environ.c
index 3145d01..131835e 100644
--- a/gdb/common/environ.c
+++ b/gdb/common/environ.c
@@ -18,165 +18,156 @@
#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)
+void
+gdb_environ::init ()
{
- struct gdb_environ *e;
+ extern char **environ;
+
+ if (environ == NULL)
+ return;
- e = XNEW (struct gdb_environ);
+ for (int i = 0; environ[i] != NULL; ++i)
+ {
+ std::string v = std::string (environ[i]);
+ std::size_t pos = v.find ('=');
+ std::string var = v.substr (0, pos);
+ std::string value = v.substr (pos + 1);
- e->allocated = 10;
- e->vector = (char **) xmalloc ((e->allocated + 1) * sizeof (char
*));
- e->vector[0] = 0;
- return e;
+ this->m_environ_map.insert (std::make_pair (var, value));
I see that make_pair has a move/xvalue version:
template< class T1, class T2 >
std::pair<V1,V2> make_pair( T1&& t, T2&& u );
Does that mean that we could/should do
this->m_environ_map.insert (std::make_pair (std::move (var), std::move
(value)));
in order to reuse the resources of the existing strings instead of
copying them?
+const char *
+gdb_environ::get (const std::string &var) const
+{
+ try
{
- e->allocated = std::max (i, e->allocated + 10);
- e->vector = (char **) xrealloc ((char *) e->vector,
- (e->allocated + 1) * sizeof (char *));
+ return (char *) this->m_environ_map.at (var).c_str ();
You can remove the cast here.
I didn't think about it at first, but some unit tests for this class
would be nice as well.
Thanks,
Simon