From patchwork Wed Jul 8 19:49:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 7590 Received: (qmail 110892 invoked by alias); 8 Jul 2015 19:49:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 110866 invoked by uid 89); 8 Jul 2015 19:49:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 08 Jul 2015 19:49:24 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id C6.00.12958.F832D955; Wed, 8 Jul 2015 15:20:15 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.98) with Microsoft SMTP Server id 14.3.210.2; Wed, 8 Jul 2015 15:49:21 -0400 Message-ID: <559D7EC1.30203@ericsson.com> Date: Wed, 8 Jul 2015 15:49:21 -0400 From: Simon Marchi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Pedro Alves , Subject: Re: [PATCH] Delete program spaces directly when removing inferiors References: <1412022790-21931-1-git-send-email-simon.marchi@ericsson.com> <559D1220.1060708@redhat.com> <559D4314.2000906@ericsson.com> <559D44FF.60705@ericsson.com> <559D48CE.5030700@redhat.com> <559D6784.9080408@ericsson.com> <559D6935.7060103@redhat.com> <559D6E89.5040709@ericsson.com> <559D7113.5000809@redhat.com> <559D7D9A.7000509@ericsson.com> In-Reply-To: <559D7D9A.7000509@ericsson.com> X-IsSubscribed: yes On 15-07-08 03:44 PM, Simon Marchi wrote: > On 15-07-08 02:50 PM, Pedro Alves wrote: >> The patch is preapproved. Meanwhile I sent a mail to gdb@ about >> deleting monitor.c and friends. > > Thanks for cleaning up the cruft. > > I pushed this updated version: > > > From 7a41607e01b505db895fcebcad618606cfab1ecf Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Wed, 8 Jul 2015 15:41:01 -0400 > Subject: [PATCH] Delete program spaces directly when removing inferiors > > When deleting an inferior, delete the associated program space as well > if it becomes unused. This replaces the "pruning" approach, with which > you could forget to call prune_program_spaces (as seen, with the > -remove-inferior command, see [1]). > > This allows to remove the prune_program_spaces function. At the same > time, I was able to clean up the delete_inferior* family: > > - delete_inferior is unused > - delete_inferior_silent is only used in monitor_close, but is replaced > with discard_all_inferiors [2], so it becomes unused > - All remaining calls to delete_inferior_1 are with silent=1, so the > parameter is removed > - delete_inferior_1 is renamed to delete_inferior > > I renamed pspace_empty_p to program_space_empty_p. I prefer if the > "exported" functions have a more explicit and standard name. > > Tested on Ubuntu 14.10. > > [1] https://sourceware.org/ml/gdb-patches/2014-09/msg00717.html > [2] See https://sourceware.org/ml/gdb-patches/2015-07/msg00228.html and > follow-ups for details. > > gdb/Changelog: > > * inferior.c (delete_inferior_1): Rename to ... > (delete_inferior): ..., remove 'silent' parameter, delete > program space when unused and remove call to prune_program_spaces. > Remove the old, unused, delete_inferior. > (delete_inferior_silent): Remove. > (prune_inferiors): Change call from delete_inferior_1 to > delete_inferior and remove 'silent' parameter. Remove call to > prune_program_spaces. > (remove_inferior_command): Idem. > * inferior.h (delete_inferior_1): Rename to... > (delete_inferior): ..., remove 'silent' parameter and remove the > original delete_inferior. > (delete_inferior_silent): Remove. > * mi/mi-main.c (mi_cmd_remove_inferior): Change call from > delete_inferior_1 to delete_inferior and remove 'silent' > parameter. > * progspace.c (prune_program_spaces): Remove. > (pspace_empty_p): Rename to... > (program_space_empty_p): ... and make non-static. > (delete_program_space): New. > * progspace.h (prune_program_spaces): Remove declaration. > (program_space_empty_p): New declaration. > (delete_program_space): New declaration. > * monitor.c (monitor_close): Replace call to > delete_thread_silent and delete_inferior_silent with > discard_all_inferiors. > --- > gdb/ChangeLog | 29 +++++++++++++++++++++++++++++ > gdb/inferior.c | 39 ++++++++------------------------------- > gdb/inferior.h | 9 +-------- > gdb/mi/mi-main.c | 2 +- > gdb/monitor.c | 3 +-- > gdb/progspace.c | 27 ++++++++++++++------------- > gdb/progspace.h | 11 +++++++---- > 7 files changed, 61 insertions(+), 59 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 3bdb099..c35cbd7 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,32 @@ > +2015-07-08 Simon Marchi > + > + * inferior.c (delete_inferior_1): Rename to ... > + (delete_inferior): ..., remove 'silent' parameter, delete > + program space when unused and remove call to prune_program_spaces. > + Remove the old, unused, delete_inferior. > + (delete_inferior_silent): Remove. > + (prune_inferiors): Change call from delete_inferior_1 to > + delete_inferior and remove 'silent' parameter. Remove call to > + prune_program_spaces. > + (remove_inferior_command): Idem. > + * inferior.h (delete_inferior_1): Rename to... > + (delete_inferior): ..., remove 'silent' parameter and remove the > + original delete_inferior. > + (delete_inferior_silent): Remove. > + * mi/mi-main.c (mi_cmd_remove_inferior): Change call from > + delete_inferior_1 to delete_inferior and remove 'silent' > + parameter. > + * progspace.c (prune_program_spaces): Remove. > + (pspace_empty_p): Rename to... > + (program_space_empty_p): ... and make non-static. > + (delete_program_space): New. > + * progspace.h (prune_program_spaces): Remove declaration. > + (program_space_empty_p): New declaration. > + (delete_program_space): New declaration. > + * monitor.c (monitor_close): Replace call to > + delete_thread_silent and delete_inferior_silent with > + discard_all_inferiors. > + > 2015-07-08 Patrick Palka > > * defs.h (deprecated_register_changed_hook): Remove prototype. > diff --git a/gdb/inferior.c b/gdb/inferior.c > index d0783d3..5e98df5 100644 > --- a/gdb/inferior.c > +++ b/gdb/inferior.c > @@ -184,11 +184,8 @@ delete_thread_of_inferior (struct thread_info *tp, void *data) > return 0; > } > > -/* If SILENT then be quiet -- don't announce a inferior death, or the > - exit of its threads. */ > - > void > -delete_inferior_1 (struct inferior *todel, int silent) > +delete_inferior (struct inferior *todel) > { > struct inferior *inf, *infprev; > struct delete_thread_of_inferior_arg arg; > @@ -203,7 +200,7 @@ delete_inferior_1 (struct inferior *todel, int silent) > return; > > arg.pid = inf->pid; > - arg.silent = silent; > + arg.silent = 1; > > iterate_over_threads (delete_thread_of_inferior, &arg); > > @@ -214,29 +211,13 @@ delete_inferior_1 (struct inferior *todel, int silent) > > observer_notify_inferior_removed (inf); > > - free_inferior (inf); > -} > - > -void > -delete_inferior (int pid) > -{ > - struct inferior *inf = find_inferior_pid (pid); > - > - delete_inferior_1 (inf, 0); > - > - if (print_inferior_events) > - printf_unfiltered (_("[Inferior %d exited]\n"), pid); > -} > + /* If this program space is rendered useless, remove it. */ > + if (program_space_empty_p (inf->pspace)) > + delete_program_space (inf->pspace); > > -void > -delete_inferior_silent (int pid) > -{ > - struct inferior *inf = find_inferior_pid (pid); > - > - delete_inferior_1 (inf, 1); > + free_inferior (inf); > } > > - > /* If SILENT then be quiet -- don't announce a inferior exit, or the > exit of its threads. */ > > @@ -509,11 +490,9 @@ prune_inferiors (void) > } > > *ss_link = ss->next; > - delete_inferior_1 (ss, 1); > + delete_inferior (ss); > ss = *ss_link; > } > - > - prune_program_spaces (); > } > > /* Simply returns the count of inferiors. */ > @@ -797,10 +776,8 @@ remove_inferior_command (char *args, int from_tty) > continue; > } > > - delete_inferior_1 (inf, 1); > + delete_inferior (inf); > } > - > - prune_program_spaces (); > } > > struct inferior * > diff --git a/gdb/inferior.h b/gdb/inferior.h > index 2054a2a..48cba45 100644 > --- a/gdb/inferior.h > +++ b/gdb/inferior.h > @@ -418,14 +418,7 @@ extern struct inferior *add_inferior (int pid); > the CLI. */ > extern struct inferior *add_inferior_silent (int pid); > > -/* Delete an existing inferior list entry, due to inferior exit. */ > -extern void delete_inferior (int pid); > - > -extern void delete_inferior_1 (struct inferior *todel, int silent); > - > -/* Same as delete_inferior, but don't print new inferior notifications > - to the CLI. */ > -extern void delete_inferior_silent (int pid); > +extern void delete_inferior (struct inferior *todel); > > /* Delete an existing inferior list entry, due to inferior detaching. */ > extern void detach_inferior (int pid); > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index ddfc9d9..66bcd88 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1964,7 +1964,7 @@ mi_cmd_remove_inferior (char *command, char **argv, int argc) > set_current_program_space (new_inferior->pspace); > } > > - delete_inferior_1 (inf, 1 /* silent */); > + delete_inferior (inf); > } > > > diff --git a/gdb/monitor.c b/gdb/monitor.c > index c7f5fc7..4657d73 100644 > --- a/gdb/monitor.c > +++ b/gdb/monitor.c > @@ -853,8 +853,7 @@ monitor_close (struct target_ops *self) > > monitor_desc = NULL; > > - delete_thread_silent (monitor_ptid); > - delete_inferior_silent (ptid_get_pid (monitor_ptid)); > + discard_all_inferiors (); > } > > /* Terminate the open connection to the remote debugger. Use this > diff --git a/gdb/progspace.c b/gdb/progspace.c > index 1c0a254..a8f5ea0 100644 > --- a/gdb/progspace.c > +++ b/gdb/progspace.c > @@ -235,8 +235,8 @@ save_current_program_space (void) > > /* Returns true iff there's no inferior bound to PSPACE. */ > > -static int > -pspace_empty_p (struct program_space *pspace) > +int > +program_space_empty_p (struct program_space *pspace) > { > if (find_inferior_for_program_space (pspace) != NULL) > return 0; > @@ -244,30 +244,31 @@ pspace_empty_p (struct program_space *pspace) > return 1; > } > > -/* Prune away automatically added program spaces that aren't required > - anymore. */ > +/* 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 > -prune_program_spaces (void) > +delete_program_space (struct program_space *pspace) > { > struct program_space *ss, **ss_link; > - struct program_space *current = current_program_space; > + gdb_assert(pspace != NULL); > + gdb_assert(pspace != current_program_space); > > ss = program_spaces; > ss_link = &program_spaces; > - while (ss) > + while (ss != NULL) > { > - if (ss == current || !pspace_empty_p (ss)) > + if (ss == pspace) > { > - ss_link = &ss->next; > - ss = *ss_link; > - continue; > + *ss_link = ss->next; > + break; > } > > - *ss_link = ss->next; > - release_program_space (ss); > + ss_link = &ss->next; > ss = *ss_link; > } > + > + release_program_space (pspace); > } > > /* Prints the list of program spaces and their details on UIOUT. If > diff --git a/gdb/progspace.h b/gdb/progspace.h > index f960093..48f206e 100644 > --- a/gdb/progspace.h > +++ b/gdb/progspace.h > @@ -236,9 +236,16 @@ extern struct program_space *current_program_space; > pointer to the new object. */ > extern struct program_space *add_program_space (struct address_space *aspace); > > +/* 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); > > +/* Returns true iff there's no inferior bound to PSPACE. */ > +extern int program_space_empty_p (struct program_space *pspace); > + > /* Copies program space SRC to DEST. Copies the main executable file, > and the main symbol file. Returns DEST. */ > extern struct program_space *clone_program_space (struct program_space *dest, > @@ -289,10 +296,6 @@ extern int address_space_num (struct address_space *aspace); > mappings. */ > extern void update_address_spaces (void); > > -/* Prune away automatically added program spaces that aren't required > - anymore. */ > -extern void prune_program_spaces (void); > - > /* Reset saved solib data at the start of an solib event. This lets > us properly collect the data when calling solib_add, so it can then > later be printed. */ > I had some formatting errors left, I pushed this right after: From 4ab31498e400c89ba9dcea2444c65aa020ab53fc Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 8 Jul 2015 15:48:02 -0400 Subject: [PATCH] Add missing spaces in previous patch gdb/ChangeLog: * progspace.c (delete_program_space): Add missing spaces. --- gdb/ChangeLog | 4 ++++ gdb/progspace.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c35cbd7..8a971e5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,9 @@ 2015-07-08 Simon Marchi + * progspace.c (delete_program_space): Add missing spaces. + +2015-07-08 Simon Marchi + * inferior.c (delete_inferior_1): Rename to ... (delete_inferior): ..., remove 'silent' parameter, delete program space when unused and remove call to prune_program_spaces. diff --git a/gdb/progspace.c b/gdb/progspace.c index a8f5ea0..ea2f8ec 100644 --- a/gdb/progspace.c +++ b/gdb/progspace.c @@ -251,8 +251,8 @@ void delete_program_space (struct program_space *pspace) { struct program_space *ss, **ss_link; - gdb_assert(pspace != NULL); - gdb_assert(pspace != current_program_space); + gdb_assert (pspace != NULL); + gdb_assert (pspace != current_program_space); ss = program_spaces; ss_link = &program_spaces;