From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71054 invoked by alias); 12 Feb 2018 18:37:10 -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 71043 invoked by uid 89); 12 Feb 2018 18:37:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-7.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,RCVD_IN_DNSWL_LOW,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=attaches, Hx-languages-length:2093 X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Feb 2018 18:37:08 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E27240FB634; Mon, 12 Feb 2018 18:37:07 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id BD4CE1006ED7; Mon, 12 Feb 2018 18:37:06 +0000 (UTC) From: Pedro Alves Subject: Re: [RFA] Convert observers to C++ To: Tom Tromey References: <20171105013039.9135-1-tom@tromey.com> <20f70b2b-bf93-a6b6-f2a4-82e14c807e05@redhat.com> <87zi4haeak.fsf@tromey.com> Cc: gdb-patches@sourceware.org Message-ID: Date: Mon, 12 Feb 2018 18:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <87zi4haeak.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-02/txt/msg00173.txt.bz2 Hi Tom, Thanks! On 02/10/2018 12:17 AM, Tom Tromey wrote: > >>> + m_observers.erase (m_observers.begin () + f); > > Pedro> Hmm, this looks incorrect. "attach" returns an index > Pedro> as token. And then detach uses "std::vector::erase()", > Pedro> which removes the element at the index and then shifts > Pedro> the rest of the elements left to fill in the space > Pedro> left over by the removed element(s). So that invalidates > Pedro> all token after the element erased. > > Yes, thanks for catching this. > > I figure that observers aren't very space-sensitive, so I just put a > counter into the observer and then put the value into the vector, like: > > std::vector> m_observers; My initial reaction was "but counter of what?", but I took a look at the patch, and I see what you mean. I'll say more in response to your new patch. > > Pedro> This could be constexpr and then defined in-class, right? > > Did it. > > Pedro> (I wonder about splitting core/lib from uses of the core. > Pedro> I.e., defining the struct observer core mechanism in one > Pedro> header, and then define the actual observers in a separate header. > Pedro> I'm basically pondering about e.g., using observers under common/ > Pedro> or in gdbserver/. Like e.g., "common/observer.h" and then > Pedro> "gdb/observers.h" and "gdbserver/observers.h", etc. But we can cross > Pedro> that bridge when we get to it, too.) > > I did this. Also I realized that calling the class "observer" is > somewhat wrong, as the callback is the observer -- so I made > common/observable.h and I renamed the class to "class observable". Yeah, the current documentation in master calls them "subjects": /* An observer is an entity who is interested in being notified when GDB reaches certain states, or certain events occur in GDB. The entity being observed is called the Subject. To receive notifications, the observer attaches a callback to the subject. One subject can have several observers. though "observable" does seem better to me. Thanks, Pedro Alves