From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 7E5B93895010 for ; Fri, 23 Jul 2021 20:46:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7E5B93895010 Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::635]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 0A2B182EC2; Fri, 23 Jul 2021 20:46:37 +0000 (UTC) Date: Fri, 23 Jul 2021 20:46:29 +0000 From: Lancelot SIX To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union Message-ID: <20210723204613.ud6kkhnt46dz4bq3@ubuntu.lan> References: <20210714045520.1623120-1-simon.marchi@polymtl.ca> <20210714045520.1623120-16-simon.marchi@polymtl.ca> <20210714120851.3pfew5pgcdp6ezn6@ubuntu.lan> <20210714171238.vzccwpurh2izbkps@ubuntu.lan> <20210714232112.wsn7pits6uuz3nf5@ubuntu.lan> <20210720230335.dcpfxbol2uwjre3b@Plymouth> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Fri, 23 Jul 2021 20:46:38 +0000 (UTC) X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Fri, 23 Jul 2021 20:46:41 -0000 Hi, I plan on spending some more time on it this week end hopefully. If you do not mind waiting a bit for my patch to be finalized, and want to go with my approach, there is indeed no point merging yours now. The main thing I try to avoid is having both of us working at the same time on different solutions of the same problem. Your approach has the advantage of being almost ready, I can wait for it to be stabilized before I adapt my work on top on it. If you are fine waiting for my approach, I’ll proceed! My plan is to: - Improve slightly https://sourceware.org/pipermail/gdb-patches/2021-July/180991.html (use gdb::Requires instead of static assert, include the python binding I forgot because I did not build it, add some selftests of the runtime checks introduced by the getters and getters, and ensure the documentation is adequate). - Adapt your patch using std::string on top on my architecture (https://sourceware.org/pipermail/gdb-patches/2021-July/180921.html). - Add a new patch that allows the use of setter / getter (based on what was is drafted in https://sourceware.org/pipermail/gdb-patches/2021-July/181037.html). As a side note, the storage of user provided getter / setter functions is done using a unions (void* is not an appropriate for that). It is possible to use the same approach to create getters and setters around the union of pointers you provide in your patch (but this would require some work, obviously). Does that makes sense? Best, Lancelot. On Fri, Jul 23, 2021 at 03:56:57PM -0400, Simon Marchi wrote: > > Hi, > > > > I reworked a bit my previous implementation to see to what extent it can > > support what you are looking for. Short answer: it can (and it is > > completely transparent to the caller). > > > > In the previous draft version[1], I introduced a struct that groups > > together the var_type and the pointer to the actual data in order to > > abstract a setting (i.e. a value with a type that can be SET or SHOWn). > > This struct provides getters and setters in order to enforce that we > > cast the pointer to the adequate type. I have modified this data > > structure so we can register user-provided callbacks. If those > > functions are provided, the getter and setters will call them to > > generate retrieve the value instead of trying to dereference the > > pointer. > > > > For the moment, I just made sure that this compiles and I played with > > dummy settings in my local tree. It is not yet finalized nor fully > > operational because do_set_command will try to free the pointer returned > > by the getter when dealing with char* data. With user provided > > callbacks memory should probably not be managed at that level but by the > > getter / setter. Nothing worrying, you have a commit that plans to > > change the char* with std::string, this should solve the issue quite > > nicely. > > > > As for the previous version, this adds a significant amount of templated > > code that can make the implementation appear quite complex. > > > > I think I’ll wait for your series to land before I finalize this > > proposal (this is just a POC for the moment) unless you want to build on > > top of this approach to introduce user provided callbacks. > > Do you suggest I merge my patch that makes it a union first? Won't that > be counter productive if you plan to make it a void* again? I'm a bit > lost now, so I am not sure what's the best order for doing things. > > Simon