From: Andrew Burgess <aburgess@redhat.com>
To: Simon Marchi <simon.marchi@efficios.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/3] gdb: rename target_waitstatus_to_string to target_waitstatus::to_string
Date: Mon, 22 Nov 2021 18:48:51 +0000 [thread overview]
Message-ID: <20211122184851.GH2514@redhat.com> (raw)
In-Reply-To: <20211122162731.3316783-1-simon.marchi@efficios.com>
* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-11-22 11:27:29 -0500]:
> Make target_waitstatus_to_string a "to_string" method of
> target_waitstatus, a bit like we have ptid_t::to_string already. This
> will save a bit of typing.
LGTM.
Thanks,
Andrew
>
> Change-Id: Id261b7a09fa9fa3c738abac131c191a6f9c13905
> ---
> gdb/gnu-nat.c | 2 +-
> gdb/infrun.c | 17 ++++++++---------
> gdb/record-btrace.c | 4 ++--
> gdb/target-debug.h | 4 +---
> gdb/target/waitstatus.c | 18 +++++++++---------
> gdb/target/waitstatus.h | 11 ++++++-----
> gdbserver/linux-low.cc | 10 +++-------
> gdbserver/server.cc | 11 +++--------
> 8 files changed, 33 insertions(+), 44 deletions(-)
>
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index c6cecff0686..216d178424f 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -1619,7 +1619,7 @@ gnu_nat_target::wait (ptid_t ptid, struct target_waitstatus *status,
>
> inf_debug (inf, "returning ptid = %s, %s",
> target_pid_to_str (ptid).c_str (),
> - target_waitstatus_to_string (status).c_str ());
> + status->to_string ().c_str ());
>
> return ptid;
> }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 89a98c20df4..dba4f33a327 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2214,7 +2214,7 @@ resume_1 (enum gdb_signal sig)
> ("thread %s has pending wait "
> "status %s (currently_stepping=%d).",
> tp->ptid.to_string ().c_str (),
> - target_waitstatus_to_string (&tp->pending_waitstatus ()).c_str (),
> + tp->pending_waitstatus ().to_string ().c_str (),
> currently_stepping (tp));
>
> tp->inf->process_target ()->threads_executing = true;
> @@ -2632,7 +2632,7 @@ clear_proceed_status_thread (struct thread_info *tp)
> infrun_debug_printf
> ("thread %s has pending wait status %s (currently_stepping=%d).",
> tp->ptid.to_string ().c_str (),
> - target_waitstatus_to_string (&tp->pending_waitstatus ()).c_str (),
> + tp->pending_waitstatus ().to_string ().c_str (),
> currently_stepping (tp));
> }
> }
> @@ -3478,7 +3478,7 @@ print_target_wait_results (ptid_t waiton_ptid, ptid_t result_ptid,
> infrun_debug_printf (" %s [%s],",
> result_ptid.to_string ().c_str (),
> target_pid_to_str (result_ptid).c_str ());
> - infrun_debug_printf (" %s", target_waitstatus_to_string (ws).c_str ());
> + infrun_debug_printf (" %s", ws->to_string ().c_str ());
> }
>
> /* Select a thread at random, out of those which are resumed and have
> @@ -3584,8 +3584,7 @@ do_target_wait_1 (inferior *inf, ptid_t ptid,
> if (tp != NULL)
> {
> infrun_debug_printf ("Using pending wait status %s for %s.",
> - target_waitstatus_to_string
> - (&tp->pending_waitstatus ()).c_str (),
> + tp->pending_waitstatus ().to_string ().c_str (),
> tp->ptid.to_string ().c_str ());
>
> /* Now that we've selected our final event LWP, un-adjust its PC
> @@ -4678,7 +4677,7 @@ static void
> save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)
> {
> infrun_debug_printf ("saving status %s for %s",
> - target_waitstatus_to_string (ws).c_str (),
> + ws->to_string ().c_str (),
> tp->ptid.to_string ().c_str ());
>
> /* Record for later. */
> @@ -4769,7 +4768,7 @@ static bool
> handle_one (const wait_one_event &event)
> {
> infrun_debug_printf
> - ("%s %s", target_waitstatus_to_string (&event.ws).c_str (),
> + ("%s %s", event.ws.to_string ().c_str (),
> event.ptid.to_string ().c_str ());
>
> if (event.ws.kind () == TARGET_WAITKIND_NO_RESUMED)
> @@ -4875,7 +4874,7 @@ handle_one (const wait_one_event &event)
>
> infrun_debug_printf
> ("target_wait %s, saving status for %s",
> - target_waitstatus_to_string (&event.ws).c_str (),
> + event.ws.to_string ().c_str (),
> t->ptid.to_string ().c_str ());
>
> /* Record for later. */
> @@ -5181,7 +5180,7 @@ handle_inferior_event (struct execution_control_state *ecs)
> end. */
> scoped_value_mark free_values;
>
> - infrun_debug_printf ("%s", target_waitstatus_to_string (&ecs->ws).c_str ());
> + infrun_debug_printf ("%s", ecs->ws.to_string ().c_str ());
>
> if (ecs->ws.kind () == TARGET_WAITKIND_IGNORE)
> {
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index a9518d57bcb..3fcfd6a4761 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -2556,7 +2556,7 @@ record_btrace_target::wait (ptid_t ptid, struct target_waitstatus *status,
> *status = btrace_step_no_resumed ();
>
> DEBUG ("wait ended by %s: %s", target_pid_to_str (null_ptid).c_str (),
> - target_waitstatus_to_string (status).c_str ());
> + status->to_string ().c_str ());
>
> return null_ptid;
> }
> @@ -2647,7 +2647,7 @@ record_btrace_target::wait (ptid_t ptid, struct target_waitstatus *status,
> DEBUG ("wait ended by thread %s (%s): %s",
> print_thread_id (eventing),
> target_pid_to_str (eventing->ptid).c_str (),
> - target_waitstatus_to_string (status).c_str ());
> + status->to_string ().c_str ());
>
> return eventing->ptid;
> }
> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
> index 5949441bc66..c9bc68b7826 100644
> --- a/gdb/target-debug.h
> +++ b/gdb/target-debug.h
> @@ -180,9 +180,7 @@
> static void
> target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status)
> {
> - std::string str = target_waitstatus_to_string (status);
> -
> - fputs_unfiltered (str.c_str (), gdb_stdlog);
> + fputs_unfiltered (status->to_string ().c_str (), gdb_stdlog);
> }
>
> \f
> diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c
> index dc3d75eef6e..2293d83230d 100644
> --- a/gdb/target/waitstatus.c
> +++ b/gdb/target/waitstatus.c
> @@ -23,40 +23,40 @@
> /* Return a pretty printed form of target_waitstatus. */
>
> std::string
> -target_waitstatus_to_string (const struct target_waitstatus *ws)
> +target_waitstatus::to_string () const
> {
> const char *kind_str = "status->kind = ";
>
> - switch (ws->kind ())
> + switch (this->kind ())
> {
> case TARGET_WAITKIND_EXITED:
> return string_printf ("%sexited, status = %d",
> - kind_str, ws->exit_status ());
> + kind_str, this->exit_status ());
>
> case TARGET_WAITKIND_STOPPED:
> return string_printf ("%sstopped, signal = %s",
> kind_str,
> - gdb_signal_to_symbol_string (ws->sig ()));
> + gdb_signal_to_symbol_string (this->sig ()));
>
> case TARGET_WAITKIND_SIGNALLED:
> return string_printf ("%ssignalled, signal = %s",
> kind_str,
> - gdb_signal_to_symbol_string (ws->sig ()));
> + gdb_signal_to_symbol_string (this->sig ()));
>
> case TARGET_WAITKIND_LOADED:
> return string_printf ("%sloaded", kind_str);
>
> case TARGET_WAITKIND_FORKED:
> return string_printf ("%sforked, child_ptid = %s", kind_str,
> - ws->child_ptid ().to_string ().c_str ());
> + this->child_ptid ().to_string ().c_str ());
>
> case TARGET_WAITKIND_VFORKED:
> return string_printf ("%svforked, child_ptid = %s", kind_str,
> - ws->child_ptid ().to_string ().c_str ());
> + this->child_ptid ().to_string ().c_str ());
>
> case TARGET_WAITKIND_EXECD:
> return string_printf ("%sexecd, execd_pathname = %s", kind_str,
> - ws->execd_pathname ());
> + this->execd_pathname ());
>
> case TARGET_WAITKIND_VFORK_DONE:
> return string_printf ("%svfork-done", kind_str);
> @@ -84,7 +84,7 @@ target_waitstatus_to_string (const struct target_waitstatus *ws)
>
> case TARGET_WAITKIND_THREAD_EXITED:
> return string_printf ("%sthread exited, status = %d",
> - kind_str, ws->exit_status ());
> + kind_str, this->exit_status ());
>
> default:
> return string_printf ("%sunknown???", kind_str);
> diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
> index 333863e6d6e..f5b050b8b82 100644
> --- a/gdb/target/waitstatus.h
> +++ b/gdb/target/waitstatus.h
> @@ -321,6 +321,12 @@ struct target_waitstatus
> return m_value.syscall_number;
> }
>
> + /* Return a pretty printed form of target_waitstatus.
> +
> + This is only meant to be used in debug messages, not for user-visible
> + messages. */
> + std::string to_string () const;
> +
> private:
> /* Reset the wait status to its original state. */
> void reset ()
> @@ -371,9 +377,4 @@ enum target_stop_reason
> TARGET_STOPPED_BY_SINGLE_STEP
> };
>
> -/* Prototypes */
> -
> -/* Return a pretty printed form of target_waitstatus. */
> -std::string target_waitstatus_to_string (const struct target_waitstatus *);
> -
> #endif /* TARGET_WAITSTATUS_H */
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index 34ede238d19..d214aff7051 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -3445,13 +3445,9 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
> if (debug_threads)
> {
> if (event_child->waitstatus.kind () != TARGET_WAITKIND_IGNORE)
> - {
> - std::string str
> - = target_waitstatus_to_string (&event_child->waitstatus);
> -
> - debug_printf ("LWP %ld: extended event with waitstatus %s\n",
> - lwpid_of (get_lwp_thread (event_child)), str.c_str ());
> - }
> + debug_printf ("LWP %ld: extended event with waitstatus %s\n",
> + lwpid_of (get_lwp_thread (event_child)),
> + event_child->waitstatus.to_string ().c_str ());
> if (current_thread->last_resume_kind == resume_step)
> {
> if (event_child->step_range_start == event_child->step_range_end)
> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
> index c58f7e01d8d..2807525d11c 100644
> --- a/gdbserver/server.cc
> +++ b/gdbserver/server.cc
> @@ -3305,14 +3305,9 @@ queue_stop_reply_callback (thread_info *thread)
> if (target_thread_stopped (thread))
> {
> if (debug_threads)
> - {
> - std::string status_string
> - = target_waitstatus_to_string (&thread->last_status);
> -
> - debug_printf ("Reporting thread %s as already stopped with %s\n",
> - target_pid_to_str (thread->id).c_str (),
> - status_string.c_str ());
> - }
> + debug_printf ("Reporting thread %s as already stopped with %s\n",
> + target_pid_to_str (thread->id).c_str (),
> + thread->last_status.to_string ().c_str ());
>
> gdb_assert (thread->last_status.kind () != TARGET_WAITKIND_IGNORE);
>
> --
> 2.33.1
>
prev parent reply other threads:[~2021-11-22 18:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-22 16:27 Simon Marchi
2021-11-22 16:27 ` [PATCH 2/3] gdb: introduce target_waitkind_str, use it in target_waitstatus::to_string Simon Marchi
2021-11-22 18:53 ` Andrew Burgess
2021-11-25 14:38 ` Vaseeharan Vinayagamoorthy
2021-11-25 15:33 ` Simon Marchi
2021-11-22 16:27 ` [PATCH 3/3] gdb: pass more const target_waitstatus by reference Simon Marchi
2021-11-22 18:56 ` Andrew Burgess
2021-11-22 18:58 ` Simon Marchi
2021-11-22 18:48 ` Andrew Burgess [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=20211122184851.GH2514@redhat.com \
--to=aburgess@redhat.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).