From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by sourceware.org (Postfix) with ESMTPS id A1B01385701F for ; Wed, 24 Mar 2021 09:29:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A1B01385701F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-ed1-x535.google.com with SMTP id j3so26763594edp.11 for ; Wed, 24 Mar 2021 02:29:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=kGUQVZIzhJgg7TnXK48qyQsDYSib5S7MdFdVI6OdYlQ=; b=dmNEXOUBscj63FwBb5e084NjC22Zhmmy5wT4iqwByiEBDHVKU4aTx9Ac8h922sXDC5 HwlDi+MCItbKUbmIkArW2aom1RRCjC8B1c6RKJ8eLF1P9Mw8/XtQL1VpnTcwRRXGBySG zX37ddazAFHxPomGJR26LqEPM/TLM5/2evrIVirSDau2VH1Qsb2TkBxxJULq61Uf1QDH p8hFdQpdsX7ySMkKI8xGRoIE08CNp90KjzRMLi7fCV9HEN1G86BpPUfiZ4uo6PhE56ew kgcze6Akol7pEUkqPtttKvgaLLKQOHUTw3OYuSjp8hUBeGleQGnveOus1oe/VfxeOvVZ u6Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kGUQVZIzhJgg7TnXK48qyQsDYSib5S7MdFdVI6OdYlQ=; b=Xl7D+wTNUmD8NhyEfDqJ/cBiaZJ/hzKEYscFCnM4LqvM+7v5QwiAJZxvvZxfbfK/3M RQxxglKvJn2W8rcNgceiEpT08F3xzkh1LHWnaITwwzOQ9WSaG9gpSrn3N7YzhnDi0xA3 1Cuzhhr8+ZE57NFXSNe0EnlHYwEN7dO2o21qrnFB/9vF2zM4THAdvdqIC1RNQ2NVvoVn 4c+XXoOUeWPcNgcKb8r4a92f0dXxb4W9OUw9afZ4CWlRpAgVpdJxQ2WhT/Djy4JggYIH C4ovVD0eiBSex3iEs0tpxfjd2na0SHS2cAxKhQzZ5gkZXjSyAszDUxHI1lFTAWgUhUAy rraw== X-Gm-Message-State: AOAM531eEyTinW1w1BULnOveioCYlCNmcq3PD/0EkBRIujlsq2huCxhj +tPmuitKS/IjsAV5JEUR/g1vUilo7bIIQw== X-Google-Smtp-Source: ABdhPJytk48rd3+e9P7hR4VxiaoJwemUlPmFdVBXS4vmllVIkKyf0n8moqg5qZxeJDTv3dUNHbNooA== X-Received: by 2002:a05:6402:8d7:: with SMTP id d23mr2444998edz.256.1616578162670; Wed, 24 Mar 2021 02:29:22 -0700 (PDT) Received: from localhost (host165-120-118-227.range165-120.btcentralplus.com. [165.120.118.227]) by smtp.gmail.com with ESMTPSA id t12sm848766edy.56.2021.03.24.02.29.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Mar 2021 02:29:22 -0700 (PDT) Date: Wed, 24 Mar 2021 09:29:21 +0000 From: Andrew Burgess To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] gdb: move all "current target" wrapper implementations to target.c Message-ID: <20210324092921.GH5520@embecosm.com> References: <20210323184138.876503-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210323184138.876503-1-simon.marchi@efficios.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 09:27:37 up 12 days, 16:39, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 24 Mar 2021 09:29:26 -0000 * Simon Marchi via Gdb-patches [2021-03-23 14:41:37 -0400]: > From: Simon Marchi > > 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