Re: [PATCH] Implement basic threading support in the NetBSD target

Tom Tromey
2020-04-01T20:43:50Z

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> Use sysctl(3) as the portable interface to prompt NetBSD threads on
Kamil> all supported NetBSD versions. In future newer versions could switch
Kamil> to PT_LWPSTATUS ptrace(2) API that will be supported on NetBSD 10.0
Kamil> and newer.

Thank you for the patch.

Kamil> +static void
Kamil> +nbsd_thread_lister (const pid_t pid,
Kamil> + std::function<bool (const struct kinfo_lwp *)> callback)

Perhaps a const reference here would be better.
Or maybe gdb::function_view, I'm not sure.

Kamil> + /* Return true if the specified thread is alive. */
Kamil> + auto lwp_alive =
Kamil> + [&l]()
Kamil> + {
Kamil> + switch (kl->l_stat)
Kamil> + {
Kamil> + case LSSLEEP:
Kamil> + case LSRUN:
Kamil> + case LSONPROC:
Kamil> + case LSSTOP:
Kamil> + case LSSUSPENDED:
Kamil> + return true;
Kamil> + default:
Kamil> + return false;
Kamil> + }
Kamil> + };
Kamil> +
Kamil> + /* Ignore embryonic or demised threads. */
Kamil> + if (!lwp_alive (l))
Kamil> + continue;

I don't think a lambda is needed here.

Kamil> +bool
Kamil> +nbsd_nat_target::thread_alive (ptid_t ptid)
Kamil> +{
Kamil> + pid_t pid = ptid.pid ();
Kamil> + int lwp = ptid.lwp ();
Kamil> + bool found = false;
Kamil> +
Kamil> + auto fn =
Kamil> + [&lwp, &found] (const struct kinfo_lwp *kl)
Kamil> + {
Kamil> + if (kl->l_lid == lwp)
Kamil> + {
Kamil> + found = true;
Kamil> + return true;
Kamil> + }
Kamil> + return false;
Kamil> + };
Kamil> +
Kamil> + nbsd_thread_lister (pid, fn);
Kamil> +
Kamil> + return found;

A classic pattern in gdb is to have one of these iteration functions
(here, nbsd_thread_lister) return bool and have it return true if any
callback returns true, and false otherwise.

That would eliminate this "found" variable.

Kamil> +const char *
Kamil> +nbsd_nat_target::thread_name (struct thread_info *thr)
Kamil> +{
Kamil> + static char buf[KI_LNAMELEN] = {};
...
Kamil> + return buf;

This should return NULL if no name was found.
The above bool return would make this easy as well.

Kamil> + fprintf_unfiltered (gdb_stdlog, "NLWP: pid=%d lwp=%d\n", pid, kl->l_lid);

This line should be removed.

Tom