From: Pedro Alves <palves@redhat.com>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH 03/28] Refactor delete_program_space as a destructor
Date: Thu, 16 Apr 2020 15:47:02 +0100 [thread overview]
Message-ID: <262059eb-49af-ee69-0f19-d3166a44a151@redhat.com> (raw)
In-Reply-To: <499a4344-14f9-d959-c0c7-af2d8a568606@simark.ca>
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)
\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 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
next prev parent reply other threads:[~2020-04-16 14:47 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-14 17:54 [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Pedro Alves
2020-04-14 17:54 ` [PATCH 01/28] Don't write to inferior_ptid in linux_get_siginfo_data Pedro Alves
2020-04-14 17:54 ` [PATCH 02/28] gcore, handle exited threads better Pedro Alves
2020-04-14 17:54 ` [PATCH 03/28] Refactor delete_program_space as a destructor Pedro Alves
2020-04-15 15:54 ` Simon Marchi
2020-04-16 14:47 ` Pedro Alves [this message]
2020-04-14 17:54 ` [PATCH 04/28] Don't write to inferior_ptid in gdbarch-selftests.c, mock address_space too Pedro Alves
2020-04-14 17:54 ` [PATCH 05/28] Don't write to inferior_ptid in inf-ptrace.c Pedro Alves
2020-04-14 17:54 ` [PATCH 06/28] Don't write to inferior_ptid in target.c Pedro Alves
2020-04-14 17:54 ` [PATCH 07/28] Don't write to inferior_ptid in infrun.c Pedro Alves
2020-04-14 17:54 ` [PATCH 08/28] Don't write to inferior_ptid in procfs.c Pedro Alves
2020-04-14 17:54 ` [PATCH 09/28] Don't write to inferior_ptid in tracefile-tfile.c Pedro Alves
2020-04-14 17:54 ` [PATCH 10/28] Don't write to inferior_ptid in tracectf.c Pedro Alves
2020-04-14 17:54 ` [PATCH 11/28] Don't write to inferior_ptid in remote.c Pedro Alves
2020-04-14 17:54 ` [PATCH 12/28] Don't write to inferior_ptid in remote-sim.c Pedro Alves
2020-04-14 17:54 ` [PATCH 13/28] Don't write to inferior_ptid in nto-procfs.c Pedro Alves
2020-04-14 17:54 ` [PATCH 14/28] Don't write to inferior_ptid in go32-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 15/28] Don't write to inferior_ptid in gnu-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 16/28] Don't write to inferior_ptid in darwin-nat.c Pedro Alves
2020-04-16 1:33 ` Simon Marchi
2020-04-16 19:23 ` Pedro Alves
2020-04-14 17:54 ` [PATCH 17/28] Don't write to inferior_ptid in corelow.c Pedro Alves
2020-04-14 17:54 ` [PATCH 18/28] Don't write to inferior_ptid in bsd-kvm.c Pedro Alves
2020-04-14 17:54 ` [PATCH 19/28] Don't write to inferior_ptid in btrace_fetch Pedro Alves
2020-04-15 4:52 ` Metzger, Markus T
2020-04-15 14:13 ` Pedro Alves
2020-04-15 15:17 ` Metzger, Markus T
2020-04-14 17:54 ` [PATCH 20/28] Don't write to inferior_ptid in bsd-kvm.c Pedro Alves
2020-04-14 17:54 ` [PATCH 21/28] Don't write to inferior_ptid in fork-child.c Pedro Alves
2020-04-14 17:54 ` [PATCH 22/28] Don't write to inferior_ptid in go32-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 23/28] Don't write to inferior_ptid in remote-sim.c Pedro Alves
2020-04-16 0:53 ` Simon Marchi
2020-04-16 14:58 ` Pedro Alves
2020-04-14 17:54 ` [PATCH 24/28] Don't write to inferior_ptid in windows-nat.c, part I Pedro Alves
2020-04-14 17:54 ` [PATCH 25/28] Don't write to inferior_ptid in windows-nat.c, part II Pedro Alves
2020-04-14 22:41 ` Hannes Domani
2020-04-15 15:08 ` Pedro Alves
2020-04-15 15:32 ` Hannes Domani
2020-04-14 17:54 ` [PATCH 26/28] Don't write to inferior_ptid in ravenscar-thread.c Pedro Alves
2020-04-17 18:45 ` Tom Tromey
2020-06-18 20:00 ` Pedro Alves
2020-06-18 21:38 ` Tom Tromey
2020-04-14 17:54 ` [PATCH 27/28] Don't write to inferior_ptid in aix-thread.c Pedro Alves
2020-04-14 17:54 ` [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Pedro Alves
2020-04-16 19:39 ` Simon Marchi
2020-04-16 20:12 ` Pedro Alves
2020-04-16 20:38 ` Simon Marchi
2020-04-17 10:29 ` Pedro Alves
2020-04-17 14:06 ` Simon Marchi
2020-04-17 16:46 ` Pedro Alves
2020-04-17 18:53 ` Tom Tromey
2020-06-18 19:59 ` Pedro Alves
2020-06-23 13:37 ` Andrew Burgess
2020-06-23 14:26 ` Pedro Alves
2020-06-23 15:38 ` [PATCH] Fix "maint selftest" regression, add struct, scoped_mock_context Pedro Alves
2020-06-23 16:34 ` Andrew Burgess
2020-06-23 17:58 ` Pedro Alves
2020-04-14 18:46 ` [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Hannes Domani
2020-04-14 19:24 ` Pedro Alves
2020-04-15 15:04 ` Simon Marchi
2020-04-16 13:41 ` Pedro Alves
2020-04-15 14:46 ` Simon Marchi
2020-04-15 15:33 ` Pedro Alves
2020-04-15 15:42 ` Simon Marchi
2020-04-17 20:20 ` Tom Tromey
2020-06-18 20:00 ` Pedro Alves
2020-06-18 22:30 ` Pedro Alves
2020-07-07 23:16 ` John Baldwin
2020-07-07 23:53 ` Pedro Alves
2020-07-08 0:19 ` John Baldwin
2020-07-08 0:10 ` Multiprocess on FreeBSD John Baldwin
2020-07-08 0:34 ` John Baldwin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=262059eb-49af-ee69-0f19-d3166a44a151@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).