From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by sourceware.org (Postfix) with ESMTPS id 4BDC83858D1E for ; Mon, 18 Apr 2022 19:01:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4BDC83858D1E 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-f49.google.com with SMTP id c10so19599126wrb.1 for ; Mon, 18 Apr 2022 12:01:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=VTOp05ujK3bwuHLCYzCIjXBUE81pKYi96zahgTYrRoA=; b=70PYkT5c08vrTQ9HVOhHHxUDOHF9UsUpKvhLc3LX96utcVF8N6gbs4VrMkevLFSiXj QBnJQLPM67hOWpAAPx3AvspPqxIf71rUOwKtBWqYU3gJXz+FmnZfYVhoG9UORdqFyzPA /M4m8Me5nf8NKxskPhV73r04c2YVv0SS0/axvTAiR9ZGOYpb/7OZ1Ku4WRLExn8AbHcz OQMToe6GEaBQZhz07YWwSeRilswyaET4sAuDDBtHIXvtwEuzvvpGk1dNb1wYYnqZ49VU jQ1XTMZTMEIQLE7vYcXh4HspNoDQVhDXY++i1yk580Lh6Dju1Rgjz/jkPzBsNxfj0tT7 8L7g== X-Gm-Message-State: AOAM531SM1H8OqO5PN/HZi7a+01SYe7ICO2IOV9R0rpzKW4rXOLOZx3P tyweD108C4Zywo4u2X+TsYI= X-Google-Smtp-Source: ABdhPJwlJQ7m1HFrVC7k6lpZ/r20OVMMnDagFvbhVEYYQWEbmnPvM5fxKz39nEOt9fo2sxQ0HYBEVA== X-Received: by 2002:adf:e194:0:b0:207:b4c9:31c6 with SMTP id az20-20020adfe194000000b00207b4c931c6mr9047249wrb.102.1650308463196; Mon, 18 Apr 2022 12:01:03 -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 z18-20020a5d6552000000b00207b65f745bsm10849919wrv.83.2022.04.18.12.01.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Apr 2022 12:01:01 -0700 (PDT) Message-ID: <08fd8bbf-c44e-7313-d7b3-7b0770c2c7d4@palves.net> Date: Mon, 18 Apr 2022 20:01:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Content-Language: en-US To: Christina Schimpe , gdb-patches@sourceware.org References: <20220329131158.3970228-1-christina.schimpe@intel.com> <20220329131158.3970228-2-christina.schimpe@intel.com> From: Pedro Alves In-Reply-To: <20220329131158.3970228-2-christina.schimpe@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Mon, 18 Apr 2022 19:01:08 -0000 On 2022-03-29 14:11, Christina Schimpe via Gdb-patches wrote: > This patch applies the appropriate FIXME notes described in commit 5b6d1e4 > "Multi-target support". > > "You'll notice that remote.c includes some FIXME notes. These refer to > the fact that the global arrays that hold data for the remote packets > supported are still globals. For example, if we connect to two > different servers/stubs, then each might support different remote > protocol features. They might even be different architectures, like > e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a > host/controller scenario as a single program. That isn't going to > work correctly today, because of said globals. I'm leaving fixing > that for another pass, since it does not appear to be trivial, and I'd > rather land the base work first. It's already useful to be able to > debug multiple instances of the same server (e.g., a distributed > cluster, where you have full control over the servers installed), so I > think as is it's already reasonable incremental progress." > > Using this patch it is possible to configure per-remote targets' > feature packets. Thanks for working on this. And so sorry for coming in late! I wish I had gotten involved in this discussion sooner. I'm not 100% sure about the solution here. What is the reasoning for making "set remote foo" affect future connections in addition to the current connection? I didn't see a discussion about that, and it seems counterintuitive to me offhand. I would think that: - if connected, the set command affects the current connection. - if not connected, the set command affects future connections. ... would be the simplest solution. Thus, if the inferior you have selected is connected and you want to configure future connections, you'd first drop the connection, or switch to an inferior that is not connected. It seems odd that set affects both current and future connections, while show only mentions the state about the current connection, for instance. Also, I think it would be better if both the set and the show commands used the same wording. Currently you have, when not connected: (gdb) set remote X-packet off Use of the 'p' packet for future remote targets is set to "off". (gdb) show remote X-packet Support for the 'p' packet on newly created remote targets is "disabled". Note the "Use of" vs "Support for", and the "for future remote targets", vs "on newly created remote targets". Also note that "disabled" is not accepted by the "set command", while printing it in quotes suggests that it would. I mean, note: Current master: (gdb) show remote X-packet Support for the `p' packet is currently disabled. (no quotes around disabled) vs your patches: (gdb) show remote X-packet Support for the 'p' packet on newly created remote targets is "disabled". and of course: (gdb) set remote X-packet disabled "on", "off" or "auto" expected. Also, BTW, err, why do we talk about the 'p' packet if the command is about the X packet? That's busted. Seems like a preexisting issue in current master. > +/* Convert the target type (future remote target or currently connected target) > + to a name used for gdb printing. */ > + > +static const char * > +get_target_type_name (bool target_connected) > +{ > + if (target_connected) > + return "on the current remote target"; > + else > + return "on newly created remote targets"; > +} Note this is not i18n friendly. Pedro Alves