[PATCH 3/3] gdb: Remove workaround for the vCont packet
Schimpe, Christina
christina.schimpe@intel.com
Fri Jan 28 13:28:55 GMT 2022
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
More information about the Gdb-patches
mailing list