[binutils-gdb] Refactor delete_program_space as a destructor

[binutils-gdb] Refactor delete_program_space as a destructor

[Pedro Alves]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=381ce63f2f010ef5c246be720ef177cf46a19179

commit 381ce63f2f010ef5c246be720ef177cf46a19179
Author: Pedro Alves <palves@redhat.com>
Date:   Thu Apr 16 14:50:07 2020 +0100

    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.

Diff:
---
 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 7ba862edd34..6c280e3f49f 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 c2e9da3d3d5..ceee00e9eed 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 13610403479..6419f017704 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -109,36 +109,65 @@ init_address_spaces (void)
 
 \f
 
-/* 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 71a6f2841e4..2b887847e17 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -209,7 +209,15 @@ private:
 
 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);