From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id DC26D3858D1E for ; Wed, 17 Aug 2022 18:09:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DC26D3858D1E Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 985651E13B; Wed, 17 Aug 2022 14:09:35 -0400 (EDT) Message-ID: Date: Wed, 17 Aug 2022 14:09:35 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH] Change bookmark allocation Content-Language: en-US To: Tom Tromey , gdb-patches@sourceware.org References: <20220815231028.352098-1-tom@tromey.com> From: Simon Marchi In-Reply-To: <20220815231028.352098-1-tom@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2022 18:09:37 -0000 On 2022-08-15 19:10, Tom Tromey wrote: > This changes how bookmarks are allocated and stored, replacing a > linked list with a vector and removing some ALL_* iterator macros. > Regression tested on x86-64 Fedora 34. > --- > gdb/reverse.c | 137 +++++++++++++++++--------------------------------- > 1 file changed, 46 insertions(+), 91 deletions(-) > > diff --git a/gdb/reverse.c b/gdb/reverse.c > index dc417497b7a..fc94b8d1825 100644 > --- a/gdb/reverse.c > +++ b/gdb/reverse.c > @@ -92,23 +92,15 @@ reverse_finish (const char *args, int from_tty) > /* Data structures for a bookmark list. */ > > struct bookmark { > - struct bookmark *next; > - int number; > - CORE_ADDR pc; > - struct symtab_and_line sal; > - gdb_byte *opaque_data; > + int number = 0; > + CORE_ADDR pc = 0; > + struct symtab_and_line sal {}; {} is unnecessary here, symtab_and_line already initializes its fields. > + gdb::unique_xmalloc_ptr opaque_data; > }; > > -static struct bookmark *bookmark_chain; > +static std::vector all_bookmarks; > static int bookmark_count; > > -#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next) > - > -#define ALL_BOOKMARKS_SAFE(B,TMP) \ > - for ((B) = bookmark_chain; \ > - (B) ? ((TMP) = (B)->next, 1) : 0; \ > - (B) = (TMP)) > - > /* save_bookmark_command -- implement "bookmark" command. > Call target method to get a bookmark identifier. > Insert bookmark identifier into list. > @@ -130,80 +122,46 @@ save_bookmark_command (const char *args, int from_tty) > error (_("target_get_bookmark failed.")); > > /* Set up a bookmark struct. */ > - bookmark *b = new bookmark (); > - b->number = ++bookmark_count; > - b->pc = regcache_read_pc (get_current_regcache ()); > - b->sal = find_pc_line (b->pc, 0); > - b->sal.pspace = get_frame_program_space (get_current_frame ()); > - b->opaque_data = bookmark_id; > - b->next = NULL; > - > - /* Add this bookmark to the end of the chain, so that a list > - of bookmarks will come out in order of increasing numbers. */ > - > - bookmark *b1 = bookmark_chain; > - if (b1 == 0) > - bookmark_chain = b; > - else > - { > - while (b1->next) > - b1 = b1->next; > - b1->next = b; > - } > - gdb_printf (_("Saved bookmark %d at %s\n"), b->number, > - paddress (gdbarch, b->sal.pc)); > + bookmark &b = all_bookmarks.emplace_back (); I don't think this works in C++11, emplace_back returns void. Otherwise, LGTM. Simon