From: "Schimpe, Christina via Gdb-patches" <gdb-patches@sourceware.org>
To: Tom Tromey <tom@tromey.com>,
Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org>
Subject: RE: [PATCH 3/3] gdb: Remove workaround for the vCont packet
Date: Fri, 28 Jan 2022 13:28:55 +0000 [thread overview]
Message-ID: <BYAPR11MB359038AC23C42DE5074A687FF9229@BYAPR11MB3590.namprd11.prod.outlook.com> (raw)
In-Reply-To: <87tue6f6jn.fsf@tromey.com>
Hi Tom,
Thanks a lot for your review.
>
> >> + /* Check vCont support and set the remote state's
> vCont_action_support
> >> + attribute. */
> >> + remote_vcont_probe ();
>
> >> - if (m_features.packet_support (PACKET_vCont) ==
> PACKET_SUPPORT_UNKNOWN)
> >> - remote_vcont_probe ();
>
> It seems like the new code should probably check whether the packet is
> supported like the old code did, in case the gdb user disabled it using the
> appropriate "set remote" command.
With the current patch, we do not check if the vCont packet is enabled when
the supports_vCont attribute is set for the target in the function
remote_vcont_probe (). We do check if the packet is enabled, when the
attribute is used later on.
To be consistent with the definition of the set/show remote commands in commit
"gdb: Make global feature array a per-remote target array", we should probably
allow the user to enable/disable the vCont packet after the connection is made
for the current target.
In this case, we have to make sure that we check the vCont packet enabling
every time the supports_vCont attribute is used.
I think for this approach, there should not be a check for packet support
before the function call remote_vcont_probe (), because the user could
enable the packet after the connection is made and then the
supports_vCont attribute should be configured correctly. So I don't think this
patch removed any of the required checks for vCont support based on the
approach I described for this patch.
But there may be other situations for which a check for vCont support is missing.
For instance, when the supports_vCont attribute is used in the function
set_range_stepping (const char *ignore_args, int from_tty,
struct cmd_list_element *c) and potentially also other places in the code.
However, I would prefer to address them in another patch.
So in case the approach I described before sounds reasonable to you,
would the current version of this patch be acceptable?
Best Regards + thanks a lot in advance,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
prev parent reply other threads:[~2022-01-28 13:29 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe via Gdb-patches
2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe via Gdb-patches
2022-01-14 19:30 ` Tom Tromey
2022-01-20 16:32 ` Schimpe, Christina via Gdb-patches
2022-01-18 11:30 ` Andrew Burgess via Gdb-patches
2022-01-20 17:24 ` Schimpe, Christina via Gdb-patches
2022-01-13 15:21 ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe via Gdb-patches
2022-01-14 19:33 ` Tom Tromey
2022-01-18 11:39 ` Andrew Burgess via Gdb-patches
2022-01-24 15:06 ` Schimpe, Christina via Gdb-patches
2022-02-04 18:03 ` Andrew Burgess via Gdb-patches
2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe via Gdb-patches
2022-01-14 19:42 ` Tom Tromey
2022-01-28 13:28 ` Schimpe, Christina via Gdb-patches [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR11MB359038AC23C42DE5074A687FF9229@BYAPR11MB3590.namprd11.prod.outlook.com \
--to=gdb-patches@sourceware.org \
--cc=christina.schimpe@intel.com \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox