This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH] Add call to prune_program_spaces in mi_cmd_remove_inferior
- From: Doug Evans <dje at google dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Sun, 28 Sep 2014 13:16:20 -0700
- Subject: Re: [PATCH] Add call to prune_program_spaces in mi_cmd_remove_inferior
- Authentication-results: sourceware.org; auth=none
- References: <1411593539-6507-1-git-send-email-simon dot marchi at ericsson dot com> <CADPb22RUCDTyQd0qtJBcJX56mpk4C_RjZn3pRobKXFHBCnc42w at mail dot gmail dot com> <54242FBD dot 7030408 at ericsson dot com>
On Thu, Sep 25, 2014 at 8:07 AM, Simon Marchi <firstname.lastname@example.org> wrote:
> On 2014-09-24 06:43 PM, Doug Evans wrote:
>> One of my pet peeves of gdb is that too much implementation logic is
>> spread throughout gdb.
>> By that I mean random bits of gdb take on the job of maintaining
>> random bits of internal gdb state,
>> instead of calling one routine (or very few) whose job it is to
>> encapsulate all that knowledge.
>> It's not clear that that applies here, but I think it does.
>> With that in mind the first question that comes to mind when reviewing
>> this patch is:
>> "Is there ever a time when deleting an inferior (from outside inferior.c)
>> would ever *not* want to also prune program spaces (at least by default)?"
> I had the same thought.
> I actually have another patch in the pipeline that addresses this. I think that
> the pruning approach is a bit wasteful when deleting an inferior. The only possible
> program space that we could possibly delete is the one that is tied to the deleted
> inferior. I was thinking of adding something like this in delete_inferior:
> /* If this program space is rendered useless, remove it. */
> if (pspace_empty_p (inf->pspace))
> delete_program_space (inf->pspace);
> (this is done after "inf" has been removed from the inferiors list, such that
> pspace_empty_p returns true if inf was the last inferior tied to that pspace)
> I think this will allow to completely remove the prune_program_spaces function,
> since deleting an inferior is the only case where this is used. If you prefer,
> I can go directly with a patch like that and drop this one. I sent the current
> one first because I thought it would be a bit more obvious and require less
> discussion (and at least get the functionality right).
That would be nice.
I only stopped where I did because I didn't want to impose vetting all
callers of delete_inferior(_*) on this patch series.
I don't have a preference, and I don't want to impose too much
specifics on the patch since you're the one writing it. OTOH, I do
want to make progress and do a bit more than your original patch.
If you're willing to go directly to always deleting unused progspaces
whenever we delete inferiors then let's do that.
btw, it would be nice to have an assert that we're not deleting the
"There's always (at least) one inferior."
This invariant should be readily visible to readers of delete_inferior(_*).
btw #2, There's also the invariant "There's always (at least) one
One is left with the question of whether they could be unrelated.
IOW could there be an inferior without a program space or a program
space without an inferior?
[At least in general. There are special cases where we do temporary
hacks to get through forks and such.]
Another cleanup could be to make this clearer.