Re: [PATCHv2 2/6] gdb: hoist target_async_permitted checks into target.c

[Simon Marchi <simark@simark.ca>]

On 2021-11-24 7:22 a.m., Andrew Burgess via Gdb-patches wrote:
> This commit moves the target_async_permitted check out of each targets
> ::can_async_p method and into the two target_can_async_p wrapper
> functions.
>
> I've left some asserts in the two ::can_async_p methods that I
> changed, which will hopefully catch any direct calls to these methods
> that might be added in the future.
>
> There should be no user visible changes after this commit.
> ---
>  gdb/linux-nat.c |  5 ++++-
>  gdb/remote.c    | 11 ++++-------
>  gdb/target.c    |  4 ++++
>  3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index f8f728481ea..3e1d1644d4c 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4088,9 +4088,12 @@ linux_nat_target::is_async_p ()
>  bool
>  linux_nat_target::can_async_p ()
>  {
> +  /* This flag should be checked in the common target.c code.  */
> +  gdb_assert (target_async_permitted);
> +
>    /* We're always async, unless the user explicitly prevented it with the
>       "maint set target-async" command.  */
> -  return target_async_permitted;
> +  return true;

The comment just above that isn't false, but maybe it's not so relevant
to have it here anymore.

>  }
>
>  bool
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 724386e0916..6ecea5b7fd7 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -14379,14 +14379,11 @@ remote_target::thread_info_to_thread_handle (struct thread_info *tp)
>  bool
>  remote_target::can_async_p ()
>  {
> -  struct remote_state *rs = get_remote_state ();
> -
> -  /* We don't go async if the user has explicitly prevented it with the
> -     "maint set target-async" command.  */
> -  if (!target_async_permitted)
> -    return false;
> +  /* This flag should be checked in the common target.c code.  */
> +  gdb_assert (target_async_permitted);
>
> -  /* We're async whenever the serial device is.  */
> +  /* We're async whenever the serial device can.  */
> +  struct remote_state *rs = get_remote_state ();
>    return serial_can_async_p (rs->remote_desc);
>  }
>
> diff --git a/gdb/target.c b/gdb/target.c
> index c5276ae0fe7..d693b670350 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -391,6 +391,8 @@ target_can_lock_scheduler ()
>  bool
>  target_can_async_p ()
>  {
> +  if (!target_async_permitted)
> +    return false;
>    return current_inferior ()->top_target ()->can_async_p ();
>  }

You could make this overload call the other one:

  return target_can_async_p (current_inferior ()->top_target ());

so that there is really a single point where target_async_permitted is
checked.

Otherwise, LGTM.

Simon