[1/3] gdbserver: Use std::list for all_dlls
Commit Message
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.
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) <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): Change parameter type to dll_info *.
(match_dll): Likewise.
(loaded_dll): Adjust to removal of entry field and all_dlls type
change.
(unloaded_dll): Adjust to all_dlls type change, use
std::find_if.
(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 | 36 ++++++++++++++++++------------------
gdb/gdbserver/dll.h | 8 +++-----
gdb/gdbserver/server.c | 20 ++++----------------
3 files changed, 25 insertions(+), 39 deletions(-)
Comments
On 10/09/2017 03:30 PM, Simon Marchi wrote:
> diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
> index 39e5eb0653..52f924bc85 100644
> --- a/gdb/gdbserver/dll.h
> +++ b/gdb/gdbserver/dll.h
> @@ -18,17 +18,15 @@
> #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;
> -
> char *name;
> CORE_ADDR base_addr;
> };
>
> -extern struct inferior_list all_dlls;
> +extern std::list<dll_info *> all_dlls;
Is there a reason for making this a list of dll_info pointers
instead of a list of dll_info objects? If you make this a list of
objects, then each list node + dll_info is allocated in one go,
very much like the current code. With a list of pointers, you
have an extra allocation/indirection for each dll_info (one for
node + pointer, another for the dll_info pointee).
Thanks,
Pedro Alves
On 2017-10-09 10:44 AM, Pedro Alves wrote:
> On 10/09/2017 03:30 PM, Simon Marchi wrote:
>
>> diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
>> index 39e5eb0653..52f924bc85 100644
>> --- a/gdb/gdbserver/dll.h
>> +++ b/gdb/gdbserver/dll.h
>> @@ -18,17 +18,15 @@
>> #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;
>> -
>> char *name;
>> CORE_ADDR base_addr;
>> };
>>
>> -extern struct inferior_list all_dlls;
>> +extern std::list<dll_info *> all_dlls;
>
> Is there a reason for making this a list of dll_info pointers
> instead of a list of dll_info objects? If you make this a list of
> objects, then each list node + dll_info is allocated in one go,
> very much like the current code. With a list of pointers, you
> have an extra allocation/indirection for each dll_info (one for
> node + pointer, another for the dll_info pointee).
No reason, except that I thought this was a reasonnable intermediary step.
I'll try doing as you say, it's probably not too long.
Simon
@@ -18,20 +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)
+free_one_dll (dll_info *dll)
{
- struct dll_info *dll = get_dll (inf);
if (dll->name != NULL)
free (dll->name);
+
free (dll);
}
@@ -39,11 +39,8 @@ free_one_dll (struct inferior_list_entry *inf)
the key is ignored; so is an all-ones base address. */
static int
-match_dll (struct inferior_list_entry *inf, void *arg)
+match_dll (dll_info *iter, dll_info *key)
{
- 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;
@@ -62,12 +59,10 @@ 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.push_back (new_dll);
dlls_changed = 1;
}
@@ -76,16 +71,19 @@ 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;
/* Be careful not to put the key DLL in any list. */
key_dll.name = (char *) name;
key_dll.base_addr = base_addr;
- dll = (struct dll_info *) find_inferior (&all_dlls, match_dll, &key_dll);
+ auto pred = [&key_dll] (dll_info *dll) {
+ return match_dll (dll, &key_dll);
+ };
- if (dll == NULL)
+ auto dll = std::find_if (all_dlls.begin (), all_dlls.end (), pred);
+
+ if (dll == 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 +97,8 @@ 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);
+ free_one_dll (*dll);
+ all_dlls.erase (dll);
dlls_changed = 1;
}
}
@@ -108,6 +106,8 @@ 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);
+ for (dll_info *dll : all_dlls)
+ free_one_dll (dll);
+
+ all_dlls.clear ();
}
@@ -18,17 +18,15 @@
#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;
-
char *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);
@@ -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, (long) dll->base_addr);
document += "</library-list>\n";