From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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 23:21:00 -0000 Message-ID: Subject: Re: [PATCH] Use std::forward_list for displaced_step_inferior_states To: Simon Marchi Cc: gdb-patches Content-Type: multipart/mixed; boundary="0000000000000369ae057b0cc8bb" X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00309.txt.bz2 --0000000000000369ae057b0cc8bb Content-Type: text/plain; charset="UTF-8" Content-length: 4751 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 > --0000000000000369ae057b0cc8bb Content-Type: text/plain; charset="US-ASCII"; name="displaced_step_list.diff" Content-Disposition: attachment; filename="displaced_step_list.diff" Content-Transfer-Encoding: base64 Content-ID: <1672c97f5dcd553be161> X-Attachment-Id: 1672c97f5dcd553be161 Content-length: 4860 ZGlmZiAtLWdpdCBnZGIvaW5mcnVuLmMgZ2RiL2luZnJ1bi5jCmluZGV4IDk0 NzNkMWYyMGYuLjg3YzIwYTc5ODIgMTAwNjQ0Ci0tLSBnZGIvaW5mcnVuLmMK KysrIGdkYi9pbmZydW4uYwpAQCAtMjEsNiArMjEsNyBAQAogI2luY2x1ZGUg ImRlZnMuaCIKICNpbmNsdWRlICJpbmZydW4uaCIKICNpbmNsdWRlIDxjdHlw ZS5oPgorI2luY2x1ZGUgPGxpc3Q+CiAjaW5jbHVkZSAic3ltdGFiLmgiCiAj aW5jbHVkZSAiZnJhbWUuaCIKICNpbmNsdWRlICJpbmZlcmlvci5oIgpAQCAt MTQ4NCw5ICsxNDg1LDYgQEAgZGlzcGxhY2VkX3N0ZXBfY2xvc3VyZTo6fmRp c3BsYWNlZF9zdGVwX2Nsb3N1cmUgKCkgPSBkZWZhdWx0OwogLyogUGVyLWlu ZmVyaW9yIGRpc3BsYWNlZCBzdGVwcGluZyBzdGF0ZS4gICovCiBzdHJ1Y3Qg ZGlzcGxhY2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGUKIHsKLSAgLyogUG9pbnRl ciB0byBuZXh0IGluIGxpbmtlZCBsaXN0LiAgKi8KLSAgc3RydWN0IGRpc3Bs YWNlZF9zdGVwX2luZmVyaW9yX3N0YXRlICpuZXh0OwotCiAgIC8qIFRoZSBw cm9jZXNzIHRoaXMgZGlzcGxhY2VkIHN0ZXAgc3RhdGUgcmVmZXJzIHRvLiAg Ki8KICAgaW5mZXJpb3IgKmluZjsKIApAQCAtMTUxNiwyMiArMTUxNCwxOCBA QCBzdHJ1Y3QgZGlzcGxhY2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGUKIAogLyog VGhlIGxpc3Qgb2Ygc3RhdGVzIG9mIHByb2Nlc3NlcyBpbnZvbHZlZCBpbiBk aXNwbGFjZWQgc3RlcHBpbmcKICAgIHByZXNlbnRseS4gICovCi1zdGF0aWMg c3RydWN0IGRpc3BsYWNlZF9zdGVwX2luZmVyaW9yX3N0YXRlICpkaXNwbGFj ZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZXM7CitzdGF0aWMgc3RkOjpsaXN0PGRp c3BsYWNlZF9zdGVwX2luZmVyaW9yX3N0YXRlPiBkaXNwbGFjZWRfc3RlcF9p bmZlcmlvcl9zdGF0ZXM7CiAKIC8qIEdldCB0aGUgZGlzcGxhY2VkIHN0ZXBw aW5nIHN0YXRlIG9mIHByb2Nlc3MgUElELiAgKi8KIAogc3RhdGljIHN0cnVj dCBkaXNwbGFjZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZSAqCiBnZXRfZGlzcGxh Y2VkX3N0ZXBwaW5nX3N0YXRlIChpbmZlcmlvciAqaW5mKQogewotICBzdHJ1 Y3QgZGlzcGxhY2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGUgKnN0YXRlOwotCi0g IGZvciAoc3RhdGUgPSBkaXNwbGFjZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZXM7 Ci0gICAgICAgc3RhdGUgIT0gTlVMTDsKLSAgICAgICBzdGF0ZSA9IHN0YXRl LT5uZXh0KQotICAgIGlmIChzdGF0ZS0+aW5mID09IGluZikKLSAgICAgIHJl dHVybiBzdGF0ZTsKKyAgZm9yIChkaXNwbGFjZWRfc3RlcF9pbmZlcmlvcl9z dGF0ZSAmc3RhdGUgOiBkaXNwbGFjZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZXMp CisgICAgaWYgKHN0YXRlLmluZiA9PSBpbmYpCisgICAgICByZXR1cm4gJnN0 YXRlOwogCi0gIHJldHVybiBOVUxMOworICByZXR1cm4gbnVsbHB0cjsKIH0K IAogLyogUmV0dXJucyB0cnVlIGlmIGFueSBpbmZlcmlvciBoYXMgYSB0aHJl YWQgZG9pbmcgYSBkaXNwbGFjZWQKQEAgLTE1NDAsMTIgKzE1MzQsOCBAQCBn ZXRfZGlzcGxhY2VkX3N0ZXBwaW5nX3N0YXRlIChpbmZlcmlvciAqaW5mKQog c3RhdGljIGludAogZGlzcGxhY2VkX3N0ZXBfaW5fcHJvZ3Jlc3NfYW55X2lu ZmVyaW9yICh2b2lkKQogewotICBzdHJ1Y3QgZGlzcGxhY2VkX3N0ZXBfaW5m ZXJpb3Jfc3RhdGUgKnN0YXRlOwotCi0gIGZvciAoc3RhdGUgPSBkaXNwbGFj ZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZXM7Ci0gICAgICAgc3RhdGUgIT0gTlVM TDsKLSAgICAgICBzdGF0ZSA9IHN0YXRlLT5uZXh0KQotICAgIGlmIChzdGF0 ZS0+c3RlcF90aHJlYWQgIT0gbnVsbHB0cikKKyAgZm9yIChkaXNwbGFjZWRf c3RlcF9pbmZlcmlvcl9zdGF0ZSAmc3RhdGUgOiBkaXNwbGFjZWRfc3RlcF9p bmZlcmlvcl9zdGF0ZXMpCisgICAgaWYgKHN0YXRlLnN0ZXBfdGhyZWFkICE9 IG51bGxwdHIpCiAgICAgICByZXR1cm4gMTsKIAogICByZXR1cm4gMDsKQEAg LTE1ODcsMjAgKzE1NzcsMTMgQEAgZGlzcGxhY2VkX3N0ZXBfaW5fcHJvZ3Jl c3MgKGluZmVyaW9yICppbmYpCiBzdGF0aWMgc3RydWN0IGRpc3BsYWNlZF9z dGVwX2luZmVyaW9yX3N0YXRlICoKIGFkZF9kaXNwbGFjZWRfc3RlcHBpbmdf c3RhdGUgKGluZmVyaW9yICppbmYpCiB7Ci0gIHN0cnVjdCBkaXNwbGFjZWRf c3RlcF9pbmZlcmlvcl9zdGF0ZSAqc3RhdGU7CisgIGZvciAoZGlzcGxhY2Vk X3N0ZXBfaW5mZXJpb3Jfc3RhdGUgJnN0YXRlIDogZGlzcGxhY2VkX3N0ZXBf aW5mZXJpb3Jfc3RhdGVzKQorICAgIGlmIChzdGF0ZS5pbmYgPT0gaW5mKQor ICAgICAgcmV0dXJuICZzdGF0ZTsKIAotICBmb3IgKHN0YXRlID0gZGlzcGxh Y2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGVzOwotICAgICAgIHN0YXRlICE9IE5V TEw7Ci0gICAgICAgc3RhdGUgPSBzdGF0ZS0+bmV4dCkKLSAgICBpZiAoc3Rh dGUtPmluZiA9PSBpbmYpCi0gICAgICByZXR1cm4gc3RhdGU7CisgIGRpc3Bs YWNlZF9zdGVwX2luZmVyaW9yX3N0YXRlcy5wdXNoX2Zyb250KHtpbmZ9KTsK IAotICBzdGF0ZSA9IFhDTkVXIChzdHJ1Y3QgZGlzcGxhY2VkX3N0ZXBfaW5m ZXJpb3Jfc3RhdGUpOwotICBzdGF0ZS0+aW5mID0gaW5mOwotICBzdGF0ZS0+ bmV4dCA9IGRpc3BsYWNlZF9zdGVwX2luZmVyaW9yX3N0YXRlczsKLSAgZGlz cGxhY2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGVzID0gc3RhdGU7Ci0KLSAgcmV0 dXJuIHN0YXRlOworICByZXR1cm4gJmRpc3BsYWNlZF9zdGVwX2luZmVyaW9y X3N0YXRlcy5mcm9udCgpOwogfQogCiAvKiBJZiBpbmZlcmlvciBpcyBpbiBk aXNwbGFjZWQgc3RlcHBpbmcsIGFuZCBBRERSIGVxdWFscyB0byBzdGFydGlu ZyBhZGRyZXNzCkBAIC0xNjI3LDI0ICsxNjEwLDE0IEBAIGdldF9kaXNwbGFj ZWRfc3RlcF9jbG9zdXJlX2J5X2FkZHIgKENPUkVfQUREUiBhZGRyKQogc3Rh dGljIHZvaWQKIHJlbW92ZV9kaXNwbGFjZWRfc3RlcHBpbmdfc3RhdGUgKGlu ZmVyaW9yICppbmYpCiB7Ci0gIHN0cnVjdCBkaXNwbGFjZWRfc3RlcF9pbmZl cmlvcl9zdGF0ZSAqaXQsICoqcHJldl9uZXh0X3A7Ci0KICAgZ2RiX2Fzc2Vy dCAoaW5mICE9IG51bGxwdHIpOwogCi0gIGl0ID0gZGlzcGxhY2VkX3N0ZXBf aW5mZXJpb3Jfc3RhdGVzOwotICBwcmV2X25leHRfcCA9ICZkaXNwbGFjZWRf c3RlcF9pbmZlcmlvcl9zdGF0ZXM7Ci0gIHdoaWxlIChpdCkKLSAgICB7Ci0g ICAgICBpZiAoaXQtPmluZiA9PSBpbmYpCi0JewotCSAgKnByZXZfbmV4dF9w ID0gaXQtPm5leHQ7Ci0JICB4ZnJlZSAoaXQpOwotCSAgcmV0dXJuOwotCX0K LQotICAgICAgcHJldl9uZXh0X3AgPSAmaXQtPm5leHQ7Ci0gICAgICBpdCA9 ICpwcmV2X25leHRfcDsKLSAgICB9CisgIGZvciAoYXV0byBJID0gZGlzcGxh Y2VkX3N0ZXBfaW5mZXJpb3Jfc3RhdGVzLmJlZ2luKCksIEUgPSBkaXNwbGFj ZWRfc3RlcF9pbmZlcmlvcl9zdGF0ZXMuZW5kKCk7IEkgIT0gRTsgKSB7Cisg ICAgaWYgKEktPmluZiA9PSBpbmYpCisgICAgICBJID0gZGlzcGxhY2VkX3N0 ZXBfaW5mZXJpb3Jfc3RhdGVzLmVyYXNlKEkpOworICAgIGVsc2UKKyAgICAg ICsrSTsKKyAgfQogfQogCiBzdGF0aWMgdm9pZAo= --0000000000000369ae057b0cc8bb--