Patchwork [v2] gdbserver: Use std::list for all_dlls

login
register
mail settings
Submitter Simon Marchi
Date Oct. 9, 2017, 3:12 p.m.
Message ID <20171009151221.7538-1-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/23412/
State New
Headers show

Comments

Simon Marchi - Oct. 9, 2017, 3:12 p.m.
As a small step towards removing inferior_list/inferior_list_entry, this
patch replaces the usage of inferior_list for the list of dlls by an
std::list.  The dll_info type now uses an std::string for name and has a
simple constructor.

I am able to build gdbserver with mingw on Linux, but I am not able to
test this on a Windows machine (the only platform that uses this code).

gdb/gdbserver/ChangeLog:

	* dll.h: Include <list>.
	(struct dll_info): Add constructor.
	<entry>: Remove field.
	(all_dlls): Change type to std::list<dll_info>.
	* dll.c: Include <algorithm>.
	(get_dll): Remove macro.
	(all_dlls): Change type to std::list<dll_info *>.
	(free_one_dll): Remove.
	(match_dll): Likewise.
	(loaded_dll): Adjust.
	(unloaded_dll): Adjust to all_dlls type change, use
	std::find_if.  Inline code from match_dll.
	(clear_dlls): Adjust to all_dlls type change.
	* server.c (emit_dll_description): Remove.
	(handle_qxfer_libraries): Adjust to all_dlls type change,
	integrate emit_dll_description's functionality.
---
 gdb/gdbserver/dll.c    | 66 ++++++++++++--------------------------------------
 gdb/gdbserver/dll.h    | 12 +++++----
 gdb/gdbserver/server.c | 20 +++------------
 3 files changed, 27 insertions(+), 71 deletions(-)
Pedro Alves - Oct. 9, 2017, 3:33 p.m.
On 10/09/2017 04:12 PM, Simon Marchi wrote:
> As a small step towards removing inferior_list/inferior_list_entry, this
> patch replaces the usage of inferior_list for the list of dlls by an
> std::list.  The dll_info type now uses an std::string for name and has a
> simple constructor.
> 
> I am able to build gdbserver with mingw on Linux, but I am not able to
> test this on a Windows machine (the only platform that uses this code).
> 

Thanks for the update.  This version is fine with me.

>  
> @@ -76,16 +40,20 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
>  void
>  unloaded_dll (const char *name, CORE_ADDR base_addr)
>  {
> -  struct dll_info *dll;
> -  struct dll_info key_dll;
> +  auto pred = [&] (const dll_info &dll) {

FWIW, I've been formatting lambdas like this:

     auto pred = [&] (const dll_info &dll)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

     for_each (..., [&] (const dll_info &dll)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

The latter models on:

     if (foo)
       {
         if (base_addr != UNSPECIFIED_CORE_ADDR

The former I think from the latter, and because that's
what emacs+tab wants.

Thanks,
Pedro Alves
Simon Marchi - Oct. 9, 2017, 3:45 p.m.
On 2017-10-09 11:33 AM, Pedro Alves wrote:
> On 10/09/2017 04:12 PM, Simon Marchi wrote:
>> As a small step towards removing inferior_list/inferior_list_entry, this
>> patch replaces the usage of inferior_list for the list of dlls by an
>> std::list.  The dll_info type now uses an std::string for name and has a
>> simple constructor.
>>
>> I am able to build gdbserver with mingw on Linux, but I am not able to
>> test this on a Windows machine (the only platform that uses this code).
>>
> 
> Thanks for the update.  This version is fine with me.
> 
>>  
>> @@ -76,16 +40,20 @@ loaded_dll (const char *name, CORE_ADDR base_addr)
>>  void
>>  unloaded_dll (const char *name, CORE_ADDR base_addr)
>>  {
>> -  struct dll_info *dll;
>> -  struct dll_info key_dll;
>> +  auto pred = [&] (const dll_info &dll) {
> 
> FWIW, I've been formatting lambdas like this:
> 
>      auto pred = [&] (const dll_info &dll)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
>      for_each (..., [&] (const dll_info &dll)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
> The latter models on:
> 
>      if (foo)
>        {
>          if (base_addr != UNSPECIFIED_CORE_ADDR
> 
> The former I think from the latter, and because that's
> what emacs+tab wants.

Ok, thanks for the tip.  I'll change that in my local branch.

Simon

Patch

diff --git a/gdb/gdbserver/dll.c b/gdb/gdbserver/dll.c
index c74601a5f4..718a36df33 100644
--- a/gdb/gdbserver/dll.c
+++ b/gdb/gdbserver/dll.c
@@ -18,56 +18,20 @@ 
 #include "server.h"
 #include "dll.h"
 
-#define get_dll(inf) ((struct dll_info *)(inf))
+#include <algorithm>
 
 /* An "unspecified" CORE_ADDR, for match_dll.  */
 #define UNSPECIFIED_CORE_ADDR (~(CORE_ADDR) 0)
 
-struct inferior_list all_dlls;
+std::list<dll_info> all_dlls;
 int dlls_changed;
 
-static void
-free_one_dll (struct inferior_list_entry *inf)
-{
-  struct dll_info *dll = get_dll (inf);
-  if (dll->name != NULL)
-    free (dll->name);
-  free (dll);
-}
-
-/* Find a DLL with the same name and/or base address.  A NULL name in
-   the key is ignored; so is an all-ones base address.  */
-
-static int
-match_dll (struct inferior_list_entry *inf, void *arg)
-{
-  struct dll_info *iter = (struct dll_info *) inf;
-  struct dll_info *key = (struct dll_info *) arg;
-
-  if (key->base_addr != UNSPECIFIED_CORE_ADDR
-      && iter->base_addr == key->base_addr)
-    return 1;
-  else if (key->name != NULL
-	   && iter->name != NULL
-	   && strcmp (key->name, iter->name) == 0)
-    return 1;
-
-  return 0;
-}
-
 /* Record a newly loaded DLL at BASE_ADDR.  */
 
 void
 loaded_dll (const char *name, CORE_ADDR base_addr)
 {
-  struct dll_info *new_dll = XCNEW (struct dll_info);
-
-  new_dll->entry.id = minus_one_ptid;
-
-  new_dll->name = xstrdup (name);
-  new_dll->base_addr = base_addr;
-
-  add_inferior_to_list (&all_dlls, &new_dll->entry);
+  all_dlls.emplace_back (name != NULL ? name : "", base_addr);
   dlls_changed = 1;
 }
 
@@ -76,16 +40,20 @@  loaded_dll (const char *name, CORE_ADDR base_addr)
 void
 unloaded_dll (const char *name, CORE_ADDR base_addr)
 {
-  struct dll_info *dll;
-  struct dll_info key_dll;
+  auto pred = [&] (const dll_info &dll) {
+    if (base_addr != UNSPECIFIED_CORE_ADDR
+        && base_addr == dll.base_addr)
+      return true;
+
+    if (name != NULL && dll.name == name)
+      return true;
 
-  /* Be careful not to put the key DLL in any list.  */
-  key_dll.name = (char *) name;
-  key_dll.base_addr = base_addr;
+    return false;
+  };
 
-  dll = (struct dll_info *) find_inferior (&all_dlls, match_dll, &key_dll);
+  auto iter = std::find_if (all_dlls.begin (), all_dlls.end (), pred);
 
-  if (dll == NULL)
+  if (iter == all_dlls.end ())
     /* For some inferiors we might get unloaded_dll events without having
        a corresponding loaded_dll.  In that case, the dll cannot be found
        in ALL_DLL, and there is nothing further for us to do.
@@ -99,8 +67,7 @@  unloaded_dll (const char *name, CORE_ADDR base_addr)
     {
       /* DLL has been found so remove the entry and free associated
          resources.  */
-      remove_inferior (&all_dlls, &dll->entry);
-      free_one_dll (&dll->entry);
+      all_dlls.erase (iter);
       dlls_changed = 1;
     }
 }
@@ -108,6 +75,5 @@  unloaded_dll (const char *name, CORE_ADDR base_addr)
 void
 clear_dlls (void)
 {
-  for_each_inferior (&all_dlls, free_one_dll);
-  clear_inferior_list (&all_dlls);
+  all_dlls.clear ();
 }
diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
index 39e5eb0653..4956efbe07 100644
--- a/gdb/gdbserver/dll.h
+++ b/gdb/gdbserver/dll.h
@@ -18,17 +18,19 @@ 
 #ifndef DLL_H
 #define DLL_H
 
+#include <list>
+
 struct dll_info
 {
-  /* This must appear first.  See inferiors.h.
-     The list iterator functions assume it.  */
-  struct inferior_list_entry entry;
+  dll_info (const std::string &name_, CORE_ADDR base_addr_)
+  : name (name_), base_addr (base_addr_)
+  {}
 
-  char *name;
+  std::string name;
   CORE_ADDR base_addr;
 };
 
-extern struct inferior_list all_dlls;
+extern std::list<dll_info> all_dlls;
 extern int dlls_changed;
 
 extern void clear_dlls (void);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a95973547c..9f0c186b86 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1533,21 +1533,6 @@  handle_qxfer_features (const char *annex,
   return len;
 }
 
-/* Worker routine for handle_qxfer_libraries.
-   Emit the XML to describe the library in INF.  */
-
-static void
-emit_dll_description (struct inferior_list_entry *inf, void *arg)
-{
-  struct dll_info *dll = (struct dll_info *) inf;
-  std::string *document = (std::string *) arg;
-  std::string name = xml_escape_text (dll->name);
-
-  *document += string_printf
-    ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
-     name.c_str (), (long) dll->base_addr);
-}
-
 /* Handle qXfer:libraries:read.  */
 
 static int
@@ -1563,7 +1548,10 @@  handle_qxfer_libraries (const char *annex,
 
   std::string document = "<library-list version=\"1.0\">\n";
 
-  for_each_inferior_with_data (&all_dlls, emit_dll_description, &document);
+  for (const dll_info &dll : all_dlls)
+    document += string_printf
+      ("  <library name=\"%s\"><segment address=\"0x%lx\"/></library>\n",
+       dll.name.c_str (), (long) dll.base_addr);
 
   document += "</library-list>\n";