From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id VHidMudBBmNPniwAWB0awg (envelope-from ) for ; Wed, 24 Aug 2022 11:21:11 -0400 Received: by simark.ca (Postfix, from userid 112) id C0CA11E4A7; Wed, 24 Aug 2022 11:21:11 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, NICE_REPLY_A autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 2F5841E21F for ; Wed, 24 Aug 2022 11:21:11 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 390FF385C335 for ; Wed, 24 Aug 2022 15:21:08 +0000 (GMT) Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) by sourceware.org (Postfix) with ESMTPS id 30AE8385C32C for ; Wed, 24 Aug 2022 15:20:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 30AE8385C32C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f42.google.com with SMTP id h24so21181573wrb.8 for ; Wed, 24 Aug 2022 08:20:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc; bh=242Oy+xclfB3wBODKbv9c3rVI99FqcHMx10ui1GdZrQ=; b=gVM/kn1hZQGnhfM0jts/ZsjzjuDmxM6VKJFVpW/Ev1JAJIXrfLtfxbI2LAyg5cKkYC QToVx1fnIen/w0bBxJ0zOZeqljoYSovIsSI1HPqb9CHcbZd/7coAF0wdIXSnZTfYTltn Zmg+DRp1gRYGnb1yZMKdU0PSqebT7lo0CUr4NmaYYXPw2P0JBAfFa2vKudWeO+d1UfA4 vwhyhnebMp6tED7y5tKci9DHPmRWYYnqYQulFX6cF6I/ok7drEE7LPkGOKiGFLs017Wx TVFjwN/1Dp3YH6oCrQ0dcpBWvNvvj9hWusVd4otm/Wag2pUVBb/byshpLi7x0RgJ+CAs uWcw== X-Gm-Message-State: ACgBeo3fGSm+ceSHGhjGBQ8Z+I7oXaSWFMxvd5SlvQ5FsHJavfjbMPPx rIQO4RGDELNFUp+hqJ0jtiA= X-Google-Smtp-Source: AA6agR7oSzaDNkNAnGs0RD8wJsc1gYMkTRRkCGx69VKEYPG8/keLj4DEUeoCkA/wyc9xhWQNEeL5/w== X-Received: by 2002:a05:6000:104f:b0:225:29be:a39c with SMTP id c15-20020a056000104f00b0022529bea39cmr16120473wrx.641.1661354454097; Wed, 24 Aug 2022 08:20:54 -0700 (PDT) Received: from ?IPv6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id l25-20020a05600c1d1900b003a62052053csm2945087wms.18.2022.08.24.08.20.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 24 Aug 2022 08:20:53 -0700 (PDT) Subject: Re: [PATCH v3 2/5] Introduce frame_info_ptr smart pointer class To: Bruno Larsen , gdb-patches@sourceware.org References: <20220725170637.79699-1-blarsen@redhat.com> <20220725170637.79699-3-blarsen@redhat.com> <67e2626e-e20c-00a2-3d6b-d58c613060af@palves.net> <1bd52cbc-71ce-672b-9815-7a485e98475a@redhat.com> From: Pedro Alves Message-ID: Date: Wed, 24 Aug 2022 16:20:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <1bd52cbc-71ce-672b-9815-7a485e98475a@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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: , Cc: Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hi Bruno, On 2022-08-24 3:24 p.m., Bruno Larsen wrote: > Hi Pedro, > > Thanks for the review, and sorry for the delayed response. > > On 25/07/2022 19:52, Pedro Alves wrote: >> On 2022-07-25 6:06 p.m., Bruno Larsen via Gdb-patches wrote: >> >>> + >>> +private: >>> + >>> +  /* The underlying pointer.  */ >>> +  frame_info *m_ptr = nullptr; >>> + >>> +  /* All frame_info_ptr objects are kept on a circular doubly-linked >>> +     list.  This keeps their construction and destruction costs >>> +     reasonably small.  To make the implementation a little simpler, >>> +     we guarantee that there is always at least one object on the list >>> +     -- this "root".  */ >> This comment is stale -- this is no longer a full frame_info object. > > I'm not sure why you mention it being a full frame_info object. I meant frame_info_ptr instead of frame_info, as that's what it was in the original version: + /* All frame_info_ptr objects are kept on a circular doubly-linked + list. This keeps their construction and destruction costs + reasonably small. To make the implementation a little simpler, + we guarantee that there is always at least one object on the list + -- this "root". */ + static frame_info_ptr root; > root was never a usable frame_info_ptr, it was just used as a pointer to the frame_info_ptr list from the outside, and to make intrusive list operations easy as we don't have to check for an empty list. I did reword the comment to: > > /* All frame_info_ptr objects are kept on an intrusive list. >   This keeps their construction and destruction costs >   reasonably small.  To make the implementation a little simpler, >   we guarantee that there is always at least one object on the list >   - this "root".  It is only used to simplify intrusive_list operations.  */ > > to hopefully explain things better. Not really. The comment was there because the linked list implementation used one element type as "root" (aka "head") object, a full frame_info_ptr object, same type as all other elements, even if only its m_next/m_prev pointers were used. This part: "To make the implementation a little simpler, we guarantee that there is always at least one object on the list this "root"." ... with the conversion to intrusive list, that comment no longer makes sense. We don't store a "root" object of element type any more. In fact, I think you should rename the "root" to frame_list, or some such, like: - static intrusive_list root; + static intrusive_list frame_list; ... and don't talk about "root" any longer; however the head node is implemented, it's an intrusive_list implementation detail.