Re: [PATCH] Delete program spaces directly when removing inferiors

Simon Marchi
2015-07-06T17:46:08Z

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);
>>> }
>>>
>>> \f
>>> 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.