From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24977 invoked by alias); 1 Dec 2014 13:25:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24953 invoked by uid 89); 1 Dec 2014 13:25:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 01 Dec 2014 13:25:24 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 0A.83.25146.8201C745; Mon, 1 Dec 2014 07:52:24 +0100 (CET) Received: from [142.133.110.254] (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.78) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 1 Dec 2014 08:25:11 -0500 Message-ID: <547C6C37.1060403@ericsson.com> Date: Mon, 01 Dec 2014 13:25:00 -0000 From: Simon Marchi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: GDB Patches Subject: Re: [PATCH] Delete program spaces directly when removing inferiors References: <1412022790-21931-1-git-send-email-simon.marchi@ericsson.com> <54454A5D.2070700@ericsson.com> In-Reply-To: <54454A5D.2070700@ericsson.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-12/txt/msg00008.txt.bz2 On 2014-10-20 01:46 PM, Simon Marchi wrote: > On 2014-09-29 04:33 PM, Simon Marchi wrote: >> 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_silent and delete_inferior were unused, which allowed >> renaming delete_inferior_1 to delete_inferior. Also, since all calls to >> it were with silent=1, I removed that parameter completely. >> >> 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. >> >> This obsoletes my previous patch "Add call to prune_program_spaces in >> mi_cmd_remove_inferior" [1]. >> >> [1] https://sourceware.org/ml/gdb-patches/2014-09/msg00717.html >> >> (err... it's the first time one of my patches has such a "big" Changelog, >> I feel like I am forgetting something.) >> >> 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. >> --- >> gdb/inferior.c | 39 ++++++++------------------------------- >> gdb/inferior.h | 9 +-------- >> gdb/mi/mi-main.c | 2 +- >> gdb/progspace.c | 38 ++++++++++++++++++++------------------ >> gdb/progspace.h | 11 +++++++---- >> 5 files changed, 37 insertions(+), 62 deletions(-) >> >> diff --git a/gdb/inferior.c b/gdb/inferior.c >> index 23da0c7..df33845 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. */ >> >> @@ -498,11 +479,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. */ >> @@ -786,10 +765,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 58557a4..67bc989 100644 >> --- a/gdb/inferior.h >> +++ b/gdb/inferior.h >> @@ -426,14 +426,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 59717ca..5d69fc7 100644 >> --- a/gdb/mi/mi-main.c >> +++ b/gdb/mi/mi-main.c >> @@ -1950,7 +1950,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/progspace.c b/gdb/progspace.c >> index b111a50..33ae6ae 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,32 @@ 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) >> + if (pspace == program_spaces) >> + program_spaces = pspace->next; >> + else >> { >> - if (ss == current || !pspace_empty_p (ss)) >> + struct program_space *i = program_spaces; >> + >> + for (i = program_spaces; i != NULL; i = i->next) >> { >> - ss_link = &ss->next; >> - ss = *ss_link; >> - continue; >> + if (i->next == pspace) >> + { >> + i->next = i->next->next; >> + break; >> + } >> } >> - >> - *ss_link = ss->next; >> - release_program_space (ss); >> - 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 08e04eb..8beebd8 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, >> @@ -288,10 +295,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. */ > > Ping. Ping.