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

[Tom Tromey <tom@tromey.com>]

>>>>> "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