From patchwork Mon Nov 19 23:21:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Blaikie X-Patchwork-Id: 30212 Received: (qmail 120257 invoked by alias); 19 Nov 2018 23:21:53 -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 120248 invoked by uid 89); 19 Nov 2018 23:21:53 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=states X-HELO: mail-it1-f193.google.com Received: from mail-it1-f193.google.com (HELO mail-it1-f193.google.com) (209.85.166.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Nov 2018 23:21:51 +0000 Received: by mail-it1-f193.google.com with SMTP id m15so732540itl.4 for ; Mon, 19 Nov 2018 15:21:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ij/Qf+p15i0rIgtd5fGuaXlx6eSWzUUa7fDPeOj76rc=; b=Pby5XYByoktfrS7RvBDe/YH+hR6P/a01lyzHjNp7YAmO1mAKZKrC4aVcv5gDGcN+EY eKsOqUuHDDWYQ/V77vfF55GzUsEjN6pS9UogYngZZSjjcGgSVk0wECFsbSGuDBWoDLH+ vMljF/7DhnkYfuYJLfVrPBbnf8uAWEZ1CKhiUiMILT+znKuKeJz/V3vGtZmDDla40kVd owiGmtXltMPn5oIksg5lyIaqd1yTcqs+KlN7JsmXVeiZJEZZsbzG44+CAw1JYjQqHO/y wVFM9CrCAxFbRc23wBufp+0SR5PLUgxLxLl9SzMMXHhMPrKDfKjS2Y0RCwZzarUyBEP/ cP5g== MIME-Version: 1.0 References: <20181112185945.24599-1-simon.marchi@ericsson.com> In-Reply-To: <20181112185945.24599-1-simon.marchi@ericsson.com> From: David Blaikie Date: Mon, 19 Nov 2018 15:21:37 -0800 Message-ID: Subject: Re: [PATCH] Use std::forward_list for displaced_step_inferior_states To: Simon Marchi Cc: gdb-patches X-IsSubscribed: yes Why forward list of pointers rather than forward list of values? Forward list of pointers would make two allocations per node, rather than one, I think? Ah, I'd replied on the other thread about this with a patch, but my email got bounced due to rich text (Google Inbox). I've attached my patch for this - though it uses list instead of forward_list - good catch on that! On Mon, Nov 12, 2018 at 11:01 AM Simon Marchi wrote: > > Use std::forward_list instead of manually implemented list. This > simplifies a bit the code, especially around removal. > > Regtested on the buildbot. There are some failures as always, but I > think they are unrelated. > > gdb/ChangeLog: > > * infrun.c (displaced_step_inferior_states): Change type to > std::forward_list. > (get_displaced_stepping_state): Adjust. > (displaced_step_in_progress_any_inferior): Adjust. > (add_displaced_stepping_state): Adjust. > (remove_displaced_stepping_state): Adjust. > --- > gdb/infrun.c | 80 +++++++++++++++++++++++----------------------------- > 1 file changed, 35 insertions(+), 45 deletions(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 9473d1f20f6..1c48740404e 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -1516,39 +1516,36 @@ struct displaced_step_inferior_state > > /* The list of states of processes involved in displaced stepping > presently. */ > -static struct displaced_step_inferior_state *displaced_step_inferior_states; > +static std::forward_list > + displaced_step_inferior_states; > > /* Get the displaced stepping state of process PID. */ > > -static struct displaced_step_inferior_state * > +static displaced_step_inferior_state * > get_displaced_stepping_state (inferior *inf) > { > - struct displaced_step_inferior_state *state; > - > - for (state = displaced_step_inferior_states; > - state != NULL; > - state = state->next) > - if (state->inf == inf) > - return state; > + for (auto *state : displaced_step_inferior_states) > + { > + if (state->inf == inf) > + return state; > + } > > - return NULL; > + return nullptr; > } > > /* Returns true if any inferior has a thread doing a displaced > step. */ > > -static int > -displaced_step_in_progress_any_inferior (void) > +static bool > +displaced_step_in_progress_any_inferior () > { > - struct displaced_step_inferior_state *state; > - > - for (state = displaced_step_inferior_states; > - state != NULL; > - state = state->next) > - if (state->step_thread != nullptr) > - return 1; > + for (auto *state : displaced_step_inferior_states) > + { > + if (state->step_thread != nullptr) > + return true; > + } > > - return 0; > + return false; > } > > /* Return true if thread represented by PTID is doing a displaced > @@ -1584,21 +1581,19 @@ displaced_step_in_progress (inferior *inf) > stepping state list, or return a pointer to an already existing > entry, if it already exists. Never returns NULL. */ > > -static struct displaced_step_inferior_state * > +static displaced_step_inferior_state * > add_displaced_stepping_state (inferior *inf) > { > - struct displaced_step_inferior_state *state; > + displaced_step_inferior_state *state > + = get_displaced_stepping_state (inf); > > - for (state = displaced_step_inferior_states; > - state != NULL; > - state = state->next) > - if (state->inf == inf) > - return state; > + if (state != nullptr) > + return state; > > state = XCNEW (struct displaced_step_inferior_state); > state->inf = inf; > - state->next = displaced_step_inferior_states; > - displaced_step_inferior_states = state; > + > + displaced_step_inferior_states.push_front (state); > > return state; > } > @@ -1627,24 +1622,19 @@ get_displaced_step_closure_by_addr (CORE_ADDR addr) > static void > remove_displaced_stepping_state (inferior *inf) > { > - struct displaced_step_inferior_state *it, **prev_next_p; > - > gdb_assert (inf != nullptr); > > - it = displaced_step_inferior_states; > - prev_next_p = &displaced_step_inferior_states; > - while (it) > - { > - if (it->inf == inf) > - { > - *prev_next_p = it->next; > - xfree (it); > - return; > - } > - > - prev_next_p = &it->next; > - it = *prev_next_p; > - } > + displaced_step_inferior_states.remove_if > + ([inf] (displaced_step_inferior_state *state) > + { > + if (state->inf == inf) > + { > + xfree (state); > + return true; > + } > + else > + return false; > + }); > } > > static void > -- > 2.19.1 > diff --git gdb/infrun.c gdb/infrun.c index 9473d1f20f..87c20a7982 100644 --- gdb/infrun.c +++ gdb/infrun.c @@ -21,6 +21,7 @@ #include "defs.h" #include "infrun.h" #include +#include #include "symtab.h" #include "frame.h" #include "inferior.h" @@ -1484,9 +1485,6 @@ displaced_step_closure::~displaced_step_closure () = default; /* Per-inferior displaced stepping state. */ struct displaced_step_inferior_state { - /* Pointer to next in linked list. */ - struct displaced_step_inferior_state *next; - /* The process this displaced step state refers to. */ inferior *inf; @@ -1516,22 +1514,18 @@ struct displaced_step_inferior_state /* The list of states of processes involved in displaced stepping presently. */ -static struct displaced_step_inferior_state *displaced_step_inferior_states; +static std::list displaced_step_inferior_states; /* Get the displaced stepping state of process PID. */ static struct displaced_step_inferior_state * get_displaced_stepping_state (inferior *inf) { - struct displaced_step_inferior_state *state; - - for (state = displaced_step_inferior_states; - state != NULL; - state = state->next) - if (state->inf == inf) - return state; + for (displaced_step_inferior_state &state : displaced_step_inferior_states) + if (state.inf == inf) + return &state; - return NULL; + return nullptr; } /* Returns true if any inferior has a thread doing a displaced @@ -1540,12 +1534,8 @@ get_displaced_stepping_state (inferior *inf) static int displaced_step_in_progress_any_inferior (void) { - struct displaced_step_inferior_state *state; - - for (state = displaced_step_inferior_states; - state != NULL; - state = state->next) - if (state->step_thread != nullptr) + for (displaced_step_inferior_state &state : displaced_step_inferior_states) + if (state.step_thread != nullptr) return 1; return 0; @@ -1587,20 +1577,13 @@ displaced_step_in_progress (inferior *inf) static struct displaced_step_inferior_state * add_displaced_stepping_state (inferior *inf) { - struct displaced_step_inferior_state *state; + for (displaced_step_inferior_state &state : displaced_step_inferior_states) + if (state.inf == inf) + return &state; - for (state = displaced_step_inferior_states; - state != NULL; - state = state->next) - if (state->inf == inf) - return state; + displaced_step_inferior_states.push_front({inf}); - state = XCNEW (struct displaced_step_inferior_state); - state->inf = inf; - state->next = displaced_step_inferior_states; - displaced_step_inferior_states = state; - - return state; + return &displaced_step_inferior_states.front(); } /* If inferior is in displaced stepping, and ADDR equals to starting address @@ -1627,24 +1610,14 @@ get_displaced_step_closure_by_addr (CORE_ADDR addr) static void remove_displaced_stepping_state (inferior *inf) { - struct displaced_step_inferior_state *it, **prev_next_p; - gdb_assert (inf != nullptr); - it = displaced_step_inferior_states; - prev_next_p = &displaced_step_inferior_states; - while (it) - { - if (it->inf == inf) - { - *prev_next_p = it->next; - xfree (it); - return; - } - - prev_next_p = &it->next; - it = *prev_next_p; - } + for (auto I = displaced_step_inferior_states.begin(), E = displaced_step_inferior_states.end(); I != E; ) { + if (I->inf == inf) + I = displaced_step_inferior_states.erase(I); + else + ++I; + } } static void