[PATCH 03/28] Refactor delete_program_space as a destructor
Pedro Alves
palves@redhat.com
Thu Apr 16 14:47:02 GMT 2020
On 4/15/20 4:54 PM, Simon Marchi wrote:
> On 2020-04-14 1:54 p.m., Pedro Alves via Gdb-patches wrote:
>> +/* Adds a new empty program space to the program space list, and binds
>> + it to ASPACE. Returns the pointer to the new object. */
>
> You could take the opportunity to update this comment. At least, the "Return the pointer"
> part is not really relevant for a constructor.
>
> Otherwise, this LGTM.
>
Thanks. Here's what I'm merging.
>From 381ce63f2f010ef5c246be720ef177cf46a19179 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 16 Apr 2020 14:50:07 +0100
Subject: [PATCH] Refactor delete_program_space as a destructor
Currently, while the program_space's ctor adds the new pspace to the
pspaces list, the destructor doesn't remove the pspace from the pspace
list. Instead, you're supposed to use delete_program_space, to both
remove the pspace from the list, and deleting the pspace.
This patch eliminates delete_program_space, and makes the pspace dtor
remove the deleted pspace from the pspace list itself, i.e., makes the
dtor do the mirror opposite of the ctor.
I found this helps with a following patch that will allocate a mock
program_space on the stack. It's easier to just let the regular dtor
remove the mock pspace from the pspace list than arrange to call
delete_program_space instead of the pspace dtor in that situation.
While at it, move the ctor/dtor intro comments to the header file, and
make the ctor explicit.
gdb/ChangeLog:
2020-04-16 Pedro Alves <palves@redhat.com>
* inferior.c (delete_inferior): Use delete operator directly
instead of delete_program_space.
* progspace.c (add_program_space): New, factored out from
program_space::program_space.
(remove_program_space): New, factored out from
delete_program_space.
(program_space::program_space): Remove intro comment. Rewrite.
(program_space::~program_space): Remove intro comment. Call
remove_program_space.
(delete_program_space): Delete.
* progspace.h (program_space::program_space): Make explicit. Move
intro comment here, adjusted.
(program_space::~program_space): Move intro comment here,
adjusted.
(delete_program_space): Remove.
---
gdb/ChangeLog | 18 +++++++++++++
gdb/inferior.c | 2 +-
gdb/progspace.c | 84 +++++++++++++++++++++++++++++----------------------------
gdb/progspace.h | 14 ++++++----
4 files changed, 71 insertions(+), 47 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7ba862edd3..6c280e3f49 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,21 @@
+2020-04-16 Pedro Alves <palves@redhat.com>
+
+ * inferior.c (delete_inferior): Use delete operator directly
+ instead of delete_program_space.
+ * progspace.c (add_program_space): New, factored out from
+ program_space::program_space.
+ (remove_program_space): New, factored out from
+ delete_program_space.
+ (program_space::program_space): Remove intro comment. Rewrite.
+ (program_space::~program_space): Remove intro comment. Call
+ remove_program_space.
+ (delete_program_space): Delete.
+ * progspace.h (program_space::program_space): Make explicit. Move
+ intro comment here, adjusted.
+ (program_space::~program_space): Move intro comment here,
+ adjusted.
+ (delete_program_space): Remove.
+
2020-04-16 Tom Tromey <tromey@adacore.com>
* windows-nat.c (windows_nat::handle_access_violation): New
diff --git a/gdb/inferior.c b/gdb/inferior.c
index c2e9da3d3d..ceee00e9ee 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -162,7 +162,7 @@ delete_inferior (struct inferior *todel)
/* If this program space is rendered useless, remove it. */
if (program_space_empty_p (inf->pspace))
- delete_program_space (inf->pspace);
+ delete inf->pspace;
delete inf;
}
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 1361040347..6419f01770 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -109,36 +109,65 @@ init_address_spaces (void)
-/* Adds a new empty program space to the program space list, and binds
- it to ASPACE. Returns the pointer to the new object. */
+/* Add a program space from the program spaces list. */
-program_space::program_space (address_space *aspace_)
-: num (++last_program_space_num), aspace (aspace_)
+static void
+add_program_space (program_space *pspace)
{
- program_space_alloc_data (this);
-
if (program_spaces == NULL)
- program_spaces = this;
+ program_spaces = pspace;
else
{
- struct program_space *last;
+ program_space *last;
for (last = program_spaces; last->next != NULL; last = last->next)
;
- last->next = this;
+ last->next = pspace;
}
}
-/* Releases program space PSPACE, and all its contents (shared
- libraries, objfiles, and any other references to the PSPACE in
- other modules). It is an internal error to call this when PSPACE
- is the current program space, since there should always be a
- program space. */
+/* Remove a program space from the program spaces list. */
+
+static void
+remove_program_space (program_space *pspace)
+{
+ program_space *ss, **ss_link;
+ gdb_assert (pspace != NULL);
+
+ ss = program_spaces;
+ ss_link = &program_spaces;
+ while (ss != NULL)
+ {
+ if (ss == pspace)
+ {
+ *ss_link = ss->next;
+ return;
+ }
+
+ ss_link = &ss->next;
+ ss = *ss_link;
+ }
+}
+
+/* See progspace.h. */
+
+program_space::program_space (address_space *aspace_)
+ : num (++last_program_space_num),
+ aspace (aspace_)
+{
+ program_space_alloc_data (this);
+
+ add_program_space (this);
+}
+
+/* See progspace.h. */
program_space::~program_space ()
{
gdb_assert (this != current_program_space);
+ remove_program_space (this);
+
scoped_restore_current_program_space restore_pspace;
set_current_program_space (this);
@@ -259,33 +288,6 @@ program_space_empty_p (struct program_space *pspace)
return 1;
}
-/* Remove a program space from the program spaces list and release it. It is
- an error to call this function while PSPACE is the current program space. */
-
-void
-delete_program_space (struct program_space *pspace)
-{
- struct program_space *ss, **ss_link;
- gdb_assert (pspace != NULL);
- gdb_assert (pspace != current_program_space);
-
- ss = program_spaces;
- ss_link = &program_spaces;
- while (ss != NULL)
- {
- if (ss == pspace)
- {
- *ss_link = ss->next;
- break;
- }
-
- ss_link = &ss->next;
- ss = *ss_link;
- }
-
- delete pspace;
-}
-
/* Prints the list of program spaces and their details on UIOUT. If
REQUESTED is not -1, it's the ID of the pspace that should be
printed. Otherwise, all spaces are printed. */
diff --git a/gdb/progspace.h b/gdb/progspace.h
index 71a6f2841e..2b887847e1 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -209,7 +209,15 @@ struct unwrapping_objfile_range
struct program_space
{
- program_space (address_space *aspace_);
+ /* Constructs a new empty program space, binds it to ASPACE, and
+ adds it to the program space list. */
+ explicit program_space (address_space *aspace);
+
+ /* Releases a program space, and all its contents (shared libraries,
+ objfiles, and any other references to the program space in other
+ modules). It is an internal error to call this when the program
+ space is the current program space, since there should always be
+ a program space. */
~program_space ();
typedef unwrapping_objfile_range objfiles_range;
@@ -362,10 +370,6 @@ extern struct program_space *current_program_space;
#define ALL_PSPACES(pspace) \
for ((pspace) = program_spaces; (pspace) != NULL; (pspace) = (pspace)->next)
-/* Remove a program space from the program spaces list and release it. It is
- an error to call this function while PSPACE is the current program space. */
-extern void delete_program_space (struct program_space *pspace);
-
/* Returns the number of program spaces listed. */
extern int number_of_program_spaces (void);
base-commit: a010605fef0eba73c564c3dd22e0a6ecbc26b10e
--
2.14.5
More information about the Gdb-patches
mailing list