From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26793 invoked by alias); 22 May 2018 03:37:35 -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 26783 invoked by uid 89); 22 May 2018 03:37:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=indirectly, closer X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 May 2018 03:37:33 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 3FC841EF62; Mon, 21 May 2018 23:37:31 -0400 (EDT) Subject: Re: [PATCH 10/10] remote: one struct remote_state per struct remote_target To: Pedro Alves , gdb-patches@sourceware.org References: <20180516141830.16859-1-palves@redhat.com> <20180516141830.16859-11-palves@redhat.com> From: Simon Marchi Message-ID: Date: Tue, 22 May 2018 05:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180516141830.16859-11-palves@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-05/txt/msg00522.txt.bz2 On 2018-05-16 10:18 AM, Pedro Alves wrote: > 'struct remote_state' today contains per-connection state, however > there's only a single global instance of that type. In order to > support multiple connections, we must have one such object per > connection. > > Thus this patch eliminates the 'remote_state' global in favor of > having a remote_state instance per remote_target instance. > > The get_remote_state free function is eliminated as well, by making it > a remote_target method instead. > > The patch then fixes the fallout by making all free functions that > refer to get_remote_state() directly or indirectly be methods of > remote_target too. > > Likewise, remote-fileio.c and remote-notif.c routines are > parameterized with a remote_target pointer too, so they can call into > the right remote_target instance. > > References to the global 'get_remote_state ()->remote_desc' to tell > whether the remote target is open (!= nullptr) must be replaced with > something else: > > - Command implementations use a new get_current_remote_target free > function. > > - remote_target::open_1 checks the exception type instead. > > Finally, remote_target and extended_remote_target are made > heap-allocated targets. As with the earlier core target patches, it > still won't be possible to have more than one remote_target instance > in practice, but this puts us closer. Hi Pedro, I took a rather quick look, because a lot of the changes are mecanical, once you have set the premises you pointed out in the commit message (and I think they are fine). Two nits: Is there a reason not to make the remote_state object a simple field of remote_target, does it have to be a pointer? You would have to shuffle things around a little bit more, but it seems to work fine. > @@ -6287,7 +6512,7 @@ remote_target::commit_resume () > we end up with too many actions for a single packet vcont_builder > flushes the current vCont packet to the remote side and starts a > new one. */ > - struct vcont_builder vcont_builder; > + struct vcont_builder vcont_builder (this); > vcont_builder.restart (); That's more a comment for the previous patch, but: I find it strange to have to call restart just after building the object. Couldn't the constructor leave it in a ready to use state? Simon