From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70765 invoked by alias); 6 Jul 2015 17:46:08 -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 70748 invoked by uid 89); 6 Jul 2015 17:46:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 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, 06 Jul 2015 17:46:04 +0000 Received: from EUSAAHC001.ericsson.se (Unknown_Domain [147.117.188.75]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id A6.A2.07675.F975A955; Mon, 6 Jul 2015 12:25:35 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.77) with Microsoft SMTP Server id 14.3.210.2; Mon, 6 Jul 2015 13:46:01 -0400 Message-ID: <559ABED9.4000207@ericsson.com> Date: Mon, 06 Jul 2015 17:46:00 -0000 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: 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> <547C6C37.1060403@ericsson.com> In-Reply-To: <547C6C37.1060403@ericsson.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00136.txt.bz2 On 14-12-01 08:25 AM, Simon Marchi wrote: > 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. > Ping.