[2/2] Remove for_each_thread
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
This patch removes the for_each_thread function, changing the callers
to use 'foreach' loops instead. In general I think loops with
iterators should be preferred over callback-based approaches -- they
are easier to read and often result in less source code as well. For
example, in this patch a helper function is inlined into its sole
caller.
---
gdb/breakpoint.c | 6 ++----
gdb/gdbthread.h | 9 ---------
gdb/infcmd.c | 55 ++++++++++++++++++++++++++-----------------------------
gdb/mi/mi-main.c | 28 +++++++++++++---------------
gdb/thread.c | 9 ---------
5 files changed, 41 insertions(+), 66 deletions(-)
Comments
On 2026-05-13 13:32, Tom Tromey wrote:
> This patch removes the for_each_thread function, changing the callers
> to use 'foreach' loops instead. In general I think loops with
> iterators should be preferred over callback-based approaches -- they
> are easier to read and often result in less source code as well. For
> example, in this patch a helper function is inlined into its sole
> caller.
Perhaps mention that there is a functional change, since you switch from
using all_threads_safe to all_threads everywhere. This happens to fix
the bug that Markus was fixing here:
https://inbox.sourceware.org/gdb-patches/20260504071636.1571615-2-markus.t.metzger@intel.com/
I would have liked to see a test along with that fix (I supplied one
down-thread), but whatever.
The other callers don't seem to need all_threads_safe, at first glance.
> @@ -607,16 +605,16 @@ print_one_inferior (struct inferior *inferior, bool recurse,
>
> if (inferior->pid != 0)
> {
> - for_each_thread ([&] (struct thread_info *ti)
> + for (auto &ti : all_threads ())
> {
> - if (ti->ptid.pid () == inferior->pid)
> + if (ti.ptid.pid () == inferior->pid)
> {
> - int core = target_core_of_thread (ti->ptid);
> + int core = target_core_of_thread (ti.ptid);
>
> if (core != -1)
> cores.insert (core);
> }
> - });
> + }
You could remove the braces of the "for" here.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Perhaps mention that there is a functional change, since you switch from
Simon> using all_threads_safe to all_threads everywhere. This happens to fix
Simon> the bug that Markus was fixing here:
Simon> https://inbox.sourceware.org/gdb-patches/20260504071636.1571615-2-markus.t.metzger@intel.com/
I was actually assuming I'd end up rebasing my patch on top of his when
it lands.
Tom
@@ -12694,10 +12694,8 @@ delete_breakpoint (struct breakpoint *bpt)
event-top.c won't do anything, and temporary breakpoints with
commands won't work. */
- for_each_thread ([&] (struct thread_info *th)
- {
- bpstat_remove_bp_location (th->control.stop_bpstat, bpt);
- });
+ for (auto &th : all_threads ())
+ bpstat_remove_bp_location (th.control.stop_bpstat, bpt);
/* Now that breakpoint is removed from breakpoint list, update the
global location list. This will remove locations that used to
@@ -791,15 +791,6 @@ extern struct thread_info *any_live_thread_of_inferior (inferior *inf);
void thread_change_ptid (process_stratum_target *targ,
ptid_t old_ptid, ptid_t new_ptid);
-/* Callback function type for function for_each_thread. */
-
-using for_each_thread_callback_ftype
- = gdb::function_view<void (thread_info *)>;
-
-/* Call CALLBACK once for each known thread. */
-
-extern void for_each_thread (for_each_thread_callback_ftype callback);
-
/* Callback function type for function find_thread. */
using find_thread_callback_ftype = gdb::function_view<bool (thread_info *)>;
@@ -685,29 +685,6 @@ starti_command (const char *args, int from_tty)
run_command_1 (args, from_tty, RUN_STOP_AT_FIRST_INSN);
}
-static void
-proceed_thread_callback (struct thread_info *thread)
-{
- /* We go through all threads individually instead of compressing
- into a single target `resume_all' request, because some threads
- may be stopped in internal breakpoints/events, or stopped waiting
- for its turn in the displaced stepping queue (that is, they are
- running from the user's perspective but internally stopped). The
- target side has no idea about why the thread is stopped, so a
- `resume_all' command would resume too much. If/when GDB gains a
- way to tell the target `hold this thread stopped until I say
- otherwise', then we can optimize this. */
- if (thread->state () != THREAD_STOPPED)
- return;
-
- if (!thread->inf->has_execution ())
- return;
-
- switch_to_thread (thread);
- clear_proceed_status (0);
- proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
-}
-
static void
ensure_valid_thread (void)
{
@@ -762,15 +739,35 @@ continue_1 (bool all_threads_p)
scoped_disable_commit_resumed disable_commit_resumed
("continue all threads in non-stop");
- for_each_thread (proceed_thread_callback);
+ for (auto &thread : all_threads ())
+ {
+ /* We go through all threads individually instead of compressing
+ into a single target `resume_all' request, because some threads
+ may be stopped in internal breakpoints/events, or stopped waiting
+ for its turn in the displaced stepping queue (that is, they are
+ running from the user's perspective but internally stopped). The
+ target side has no idea about why the thread is stopped, so a
+ `resume_all' command would resume too much. If/when GDB gains a
+ way to tell the target `hold this thread stopped until I say
+ otherwise', then we can optimize this. */
+ if (thread.state () != THREAD_STOPPED)
+ continue;
+
+ if (!thread.inf->has_execution ())
+ continue;
+
+ switch_to_thread (&thread);
+ clear_proceed_status (0);
+ proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
+ }
if (current_ui->prompt_state == PROMPT_BLOCKED)
{
- /* If all threads in the target were already running,
- proceed_thread_callback ends up never calling proceed,
- and so nothing calls this to put the inferior's terminal
- settings in effect and remove stdin from the event loop,
- which we must when running a foreground command. E.g.:
+ /* If all threads in the target were already running, the
+ above ends up never calling proceed, and so nothing calls
+ this to put the inferior's terminal settings in effect
+ and remove stdin from the event loop, which we must when
+ running a foreground command. E.g.:
(gdb) c -a&
Continuing.
@@ -278,10 +278,8 @@ exec_continue (const char *const *argv, int argc)
pid = inf->pid;
}
- for_each_thread ([&] (struct thread_info *thread)
- {
- proceed_thread (thread, pid);
- });
+ for (auto &thread : all_threads ())
+ proceed_thread (&thread, pid);
disable_commit_resumed.reset_and_commit ();
}
else
@@ -363,16 +361,16 @@ mi_cmd_exec_interrupt (const char *command, const char *const *argv, int argc)
scoped_disable_commit_resumed disable_commit_resumed
("interrupting all threads of thread group");
- for_each_thread ([&] (struct thread_info *thread)
+ for (auto &thread : all_threads ())
{
- if (thread->state () != THREAD_RUNNING)
- return;
+ if (thread.state () != THREAD_RUNNING)
+ continue;
- if (thread->ptid.pid () != inf->pid)
- return;
+ if (thread.ptid.pid () != inf->pid)
+ continue;
- target_stop (thread->ptid);
- });
+ target_stop (thread.ptid);
+ }
}
else
{
@@ -607,16 +605,16 @@ print_one_inferior (struct inferior *inferior, bool recurse,
if (inferior->pid != 0)
{
- for_each_thread ([&] (struct thread_info *ti)
+ for (auto &ti : all_threads ())
{
- if (ti->ptid.pid () == inferior->pid)
+ if (ti.ptid.pid () == inferior->pid)
{
- int core = target_core_of_thread (ti->ptid);
+ int core = target_core_of_thread (ti.ptid);
if (core != -1)
cores.insert (core);
}
- });
+ }
}
if (!cores.empty ())
@@ -602,15 +602,6 @@ find_thread_by_handle (gdb::array_view<const gdb_byte> handle,
/* See gdbthread.h. */
-void
-for_each_thread (for_each_thread_callback_ftype callback)
-{
- for (thread_info &tp : all_threads_safe ())
- callback (&tp);
-}
-
-/* See gdbthread.h. */
-
struct thread_info *
find_thread (find_thread_callback_ftype callback)
{