From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Simon Marchi <simon.marchi@efficios.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/2] gdb: move all "current target" wrapper implementations to target.c
Date: Wed, 24 Mar 2021 09:29:21 +0000 [thread overview]
Message-ID: <20210324092921.GH5520@embecosm.com> (raw)
In-Reply-To: <20210323184138.876503-1-simon.marchi@efficios.com>
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-03-23 14:41:37 -0400]:
> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> The following patch removes the current_top_target function, replacing
> uses with `current_inferior ()->top_target ()`. This is a problem for
> uses in target.h, because they don't have access to the current_inferior
> function and the inferior structure: target.h can't include inferior.h,
> otherwise that would make a cyclic inclusion.
>
> Avoid this by moving all implementations of the wrappers that call
> target methods with the current target to target.c. Many of them are
> changed from a macro to a function, which is an improvement for
> readability and debuggability, IMO.
>
> target_shortname and target_longname were not function-like macros, so a
> few adjustments are needed.
This all makes sense and seems like an improvement. I had just one
observation...
>
> gdb/ChangeLog:
>
> * target.h (target_shortname): Change to function declaration.
> (target_longname): Likewise.
> (target_attach_no_wait): Likewise.
> (target_post_attach): Likewise.
> (target_prepare_to_store): Likewise.
> (target_supports_enable_disable_tracepoint): Likewise.
> (target_supports_string_tracing): Likewise.
> (target_supports_evaluation_of_breakpoint_conditions): Likewise.
> (target_supports_dumpcore): Likewise.
> (target_dumpcore): Likewise.
> (target_can_run_breakpoint_commands): Likewise.
> (target_files_info): Likewise.
> (target_post_startup_inferior): Likewise.
> (target_insert_fork_catchpoint): Likewise.
> (target_remove_fork_catchpoint): Likewise.
> (target_insert_vfork_catchpoint): Likewise.
> (target_remove_vfork_catchpoint): Likewise.
> (target_insert_exec_catchpoint): Likewise.
> (target_remove_exec_catchpoint): Likewise.
> (target_set_syscall_catchpoint): Likewise.
> (target_rcmd): Likewise.
> (target_can_lock_scheduler): Likewise.
> (target_can_async_p): Likewise.
> (target_is_async_p): Likewise.
> (target_execution_direction): Likewise.
> (target_extra_thread_info): Likewise.
> (target_pid_to_exec_file): Likewise.
> (target_thread_architecture): Likewise.
> (target_find_memory_regions): Likewise.
> (target_make_corefile_notes): Likewise.
> (target_get_bookmark): Likewise.
> (target_goto_bookmark): Likewise.
> (target_stopped_by_watchpoint): Likewise.
> (target_stopped_by_sw_breakpoint): Likewise.
> (target_supports_stopped_by_sw_breakpoint): Likewise.
> (target_stopped_by_hw_breakpoint): Likewise.
> (target_supports_stopped_by_hw_breakpoint): Likewise.
> (target_have_steppable_watchpoint): Likewise.
> (target_can_use_hardware_watchpoint): Likewise.
> (target_region_ok_for_hw_watchpoint): Likewise.
> (target_can_do_single_step): Likewise.
> (target_insert_watchpoint): Likewise.
> (target_remove_watchpoint): Likewise.
> (target_insert_hw_breakpoint): Likewise.
> (target_remove_hw_breakpoint): Likewise.
> (target_can_accel_watchpoint_condition): Likewise.
> (target_can_execute_reverse): Likewise.
> (target_get_ada_task_ptid): Likewise.
> (target_filesystem_is_local): Likewise.
> (target_trace_init): Likewise.
> (target_download_tracepoint): Likewise.
> (target_can_download_tracepoint): Likewise.
> (target_download_trace_state_variable): Likewise.
> (target_enable_tracepoint): Likewise.
> (target_disable_tracepoint): Likewise.
> (target_trace_start): Likewise.
> (target_trace_set_readonly_regions): Likewise.
> (target_get_trace_status): Likewise.
> (target_get_tracepoint_status): Likewise.
> (target_trace_stop): Likewise.
> (target_trace_find): Likewise.
> (target_get_trace_state_variable_value): Likewise.
> (target_save_trace_data): Likewise.
> (target_upload_tracepoints): Likewise.
> (target_upload_trace_state_variables): Likewise.
> (target_get_raw_trace_data): Likewise.
> (target_get_min_fast_tracepoint_insn_len): Likewise.
> (target_set_disconnected_tracing): Likewise.
> (target_set_circular_trace_buffer): Likewise.
> (target_set_trace_buffer_size): Likewise.
> (target_set_trace_notes): Likewise.
> (target_get_tib_address): Likewise.
> (target_set_permissions): Likewise.
> (target_static_tracepoint_marker_at): Likewise.
> (target_static_tracepoint_markers_by_strid): Likewise.
> (target_traceframe_info): Likewise.
> (target_use_agent): Likewise.
> (target_can_use_agent): Likewise.
> (target_augmented_libraries_svr4_read): Likewise.
> (target_log_command): Likewise.
> * target.c (target_shortname): New.
> (target_longname): New.
> (target_attach_no_wait): New.
> (target_post_attach): New.
> (target_prepare_to_store): New.
> (target_supports_enable_disable_tracepoint): New.
> (target_supports_string_tracing): New.
> (target_supports_evaluation_of_breakpoint_conditions): New.
> (target_supports_dumpcore): New.
> (target_dumpcore): New.
> (target_can_run_breakpoint_commands): New.
> (target_files_info): New.
> (target_post_startup_inferior): New.
> (target_insert_fork_catchpoint): New.
> (target_remove_fork_catchpoint): New.
> (target_insert_vfork_catchpoint): New.
> (target_remove_vfork_catchpoint): New.
> (target_insert_exec_catchpoint): New.
> (target_remove_exec_catchpoint): New.
> (target_set_syscall_catchpoint): New.
> (target_rcmd): New.
> (target_can_lock_scheduler): New.
> (target_can_async_p): New.
> (target_is_async_p): New.
> (target_execution_direction): New.
> (target_extra_thread_info): New.
> (target_pid_to_exec_file): New.
> (target_thread_architecture): New.
> (target_find_memory_regions): New.
> (target_make_corefile_notes): New.
> (target_get_bookmark): New.
> (target_goto_bookmark): New.
> (target_stopped_by_watchpoint): New.
> (target_stopped_by_sw_breakpoint): New.
> (target_supports_stopped_by_sw_breakpoint): New.
> (target_stopped_by_hw_breakpoint): New.
> (target_supports_stopped_by_hw_breakpoint): New.
> (target_have_steppable_watchpoint): New.
> (target_can_use_hardware_watchpoint): New.
> (target_region_ok_for_hw_watchpoint): New.
> (target_can_do_single_step): New.
> (target_insert_watchpoint): New.
> (target_remove_watchpoint): New.
> (target_insert_hw_breakpoint): New.
> (target_remove_hw_breakpoint): New.
> (target_can_accel_watchpoint_condition): New.
> (target_can_execute_reverse): New.
> (target_get_ada_task_ptid): New.
> (target_filesystem_is_local): New.
> (target_trace_init): New.
> (target_download_tracepoint): New.
> (target_can_download_tracepoint): New.
> (target_download_trace_state_variable): New.
> (target_enable_tracepoint): New.
> (target_disable_tracepoint): New.
> (target_trace_start): New.
> (target_trace_set_readonly_regions): New.
> (target_get_trace_status): New.
> (target_get_tracepoint_status): New.
> (target_trace_stop): New.
> (target_trace_find): New.
> (target_get_trace_state_variable_value): New.
> (target_save_trace_data): New.
> (target_upload_tracepoints): New.
> (target_upload_trace_state_variables): New.
> (target_get_raw_trace_data): New.
> (target_get_min_fast_tracepoint_insn_len): New.
> (target_set_disconnected_tracing): New.
> (target_set_circular_trace_buffer): New.
> (target_set_trace_buffer_size): New.
> (target_set_trace_notes): New.
> (target_get_tib_address): New.
> (target_set_permissions): New.
> (target_static_tracepoint_marker_at): New.
> (target_static_tracepoint_markers_by_strid): New.
> (target_traceframe_info): New.
> (target_use_agent): New.
> (target_can_use_agent): New.
> (target_augmented_libraries_svr4_read): New.
> (target_log_command): New.
> * bfin-tdep.c (bfin_sw_breakpoint_from_kind): Adjust.
> * infrun.c (set_schedlock_func): Adjust.
> * mi/mi-main.c (exec_reverse_continue): Adjust.
> * reverse.c (exec_reverse_once): Adjust.
> * sh-tdep.c (sh_sw_breakpoint_from_kind): Adjust.
> * tui/tui-stack.c (tui_locator_window::make_status_line): Adjust.
> * remote-sim.c (gdbsim_target::detach): Adjust.
> (gdbsim_target::files_info): Adjust.
>
> Change-Id: I72ef56e9a25adeb0b91f1ad05e34c89f77ebeaa8
> ---
> gdb/bfin-tdep.c | 2 +-
> gdb/infrun.c | 3 +-
> gdb/mi/mi-main.c | 2 +-
> gdb/remote-sim.c | 4 +-
> gdb/reverse.c | 2 +-
> gdb/sh-tdep.c | 2 +-
> gdb/target.c | 558 ++++++++++++++++++++++++++++++++++++++++++++
> gdb/target.h | 287 +++++++++--------------
> gdb/tui/tui-stack.c | 4 +-
> 9 files changed, 674 insertions(+), 190 deletions(-)
>
> diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
> index 963b0485a48..e3d6eee8832 100644
> --- a/gdb/bfin-tdep.c
> +++ b/gdb/bfin-tdep.c
> @@ -597,7 +597,7 @@ bfin_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)
>
> *size = kind;
>
> - if (strcmp (target_shortname, "sim") == 0)
> + if (strcmp (target_shortname (), "sim") == 0)
> return bfin_sim_breakpoint;
> else
> return bfin_breakpoint;
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 50340e6edad..b6f399d5478 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2030,7 +2030,8 @@ set_schedlock_func (const char *args, int from_tty, struct cmd_list_element *c)
> if (!target_can_lock_scheduler ())
> {
> scheduler_mode = schedlock_off;
> - error (_("Target '%s' cannot support this command."), target_shortname);
> + error (_("Target '%s' cannot support this command."),
> + target_shortname ());
> }
> }
>
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 9a14d78e1e2..d5ce08e95ed 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -322,7 +322,7 @@ exec_reverse_continue (char **argv, int argc)
> error (_("Already in reverse mode."));
>
> if (!target_can_execute_reverse ())
> - error (_("Target %s does not support this command."), target_shortname);
> + error (_("Target %s does not support this command."), target_shortname ());
>
> scoped_restore save_exec_dir = make_scoped_restore (&execution_direction,
> EXEC_REVERSE);
> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index 1746b626fa1..f2cf8dbd711 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -836,7 +836,7 @@ gdbsim_target::detach (inferior *inf, int from_tty)
>
> inf->unpush_target (this); /* calls gdbsim_close to do the real work */
> if (from_tty)
> - printf_filtered ("Ending simulator %s debugging\n", target_shortname);
> + printf_filtered ("Ending simulator %s debugging\n", target_shortname ());
> }
>
> /* Resume execution of the target process. STEP says whether to single-step
> @@ -1119,7 +1119,7 @@ gdbsim_target::files_info ()
> if (current_program_space->exec_bfd ())
> {
> fprintf_unfiltered (gdb_stdlog, "\tAttached to %s running program %s\n",
> - target_shortname, file);
> + target_shortname (), file);
> sim_info (sim_data->gdbsim_desc, 0);
> }
> }
> diff --git a/gdb/reverse.c b/gdb/reverse.c
> index 4c618a00173..e51defb27a3 100644
> --- a/gdb/reverse.c
> +++ b/gdb/reverse.c
> @@ -45,7 +45,7 @@ exec_reverse_once (const char *cmd, const char *args, int from_tty)
> cmd);
>
> if (!target_can_execute_reverse ())
> - error (_("Target %s does not support this command."), target_shortname);
> + error (_("Target %s does not support this command."), target_shortname ());
>
> std::string reverse_command = string_printf ("%s %s", cmd, args ? args : "");
> scoped_restore restore_exec_dir
> diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
> index ecd73c17217..5a71d3ac689 100644
> --- a/gdb/sh-tdep.c
> +++ b/gdb/sh-tdep.c
> @@ -432,7 +432,7 @@ sh_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)
> *size = kind;
>
> /* For remote stub targets, trapa #20 is used. */
> - if (strcmp (target_shortname, "remote") == 0)
> + if (strcmp (target_shortname (), "remote") == 0)
> {
> static unsigned char big_remote_breakpoint[] = { 0xc3, 0x20 };
> static unsigned char little_remote_breakpoint[] = { 0x20, 0xc3 };
> diff --git a/gdb/target.c b/gdb/target.c
> index afc4b2cbbb6..e7af5e8a9b0 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -215,6 +215,564 @@ target_has_execution (inferior *inf)
> return false;
> }
>
> +const char *
> +target_shortname ()
There are a few of the functions here missing a header comment.
Otherwise, this all looks fine.
Thanks,
Andrew
next prev parent reply other threads:[~2021-03-24 9:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-23 18:41 Simon Marchi
2021-03-23 18:41 ` [PATCH 2/2] gdb: remove current_top_target function Simon Marchi
2021-03-24 11:02 ` Andrew Burgess
2021-03-24 22:11 ` Simon Marchi
2021-03-24 9:29 ` Andrew Burgess [this message]
2021-03-24 14:04 ` [PATCH 1/2] gdb: move all "current target" wrapper implementations to target.c Simon Marchi
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=20210324092921.GH5520@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).