Re: [PATCH] gdb: fix darwin-nat.c build / adapt to multi-target

[Pedro Alves <palves@redhat.com>]

On 1/23/20 4:59 PM, Simon Marchi wrote:
> From: Simon Marchi <smarchi@dev-macosx-01.internal.efficios.com>
> 
> The darwin-nat.c file doesn't build since the multi-target changes
> (5b6d1e4f, "Multi-target support").  This patch makes it build.  I have
> access to a macOS vm, so I am able to build it, but I wasn't able to
> successfully codesign it and try to actually debug something, so I don't
> know if it works.  I don't have much more time to put on this to figure
> it out, so I thought I'd sent the patch anyway, as it's at least a step
> in the right direction.
> 
> The bulk of the patch is to change a bunch of functions to be methods of
> the darwin_nat_target object, so that this can pass `this` to
> find_inferior_ptid and other functions that now require a
> process_stratum_target pointer.
> 
> The darwin_ptrace_him function (renamed to darwin_nat_target::ptrace_him
> in this patch) is passed to fork_inferior as the `init_trace_fun`
> parameter.  Since the method can't be passed as a plain function pointer
> (we need the `this` pointer), I changed the `init_trace_fun` parameter
> of fork_inferior to be a gdb::function_view, so we can pass a lambda and
> capture `this`.

This LGTM.  Thanks for doing this.  

The use of function_view infork_inferior gave me pause, but it's safe
since fork_inferior doesn't return until the child either exec'ed or
exited -- there's no risk that the fork parent returns from the current
function before the fork child accesses the function_view's closure.

Note that this larger patch wasn't really necessary, since for native
targets there's only ever one process_stratum_target object.  So you
could instead have referred to the global where you need it.
But your patch is better.

> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 78f972a7496d..ef5a75292fb1 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -267,7 +267,8 @@ execv_argv::init_for_shell (const char *exec_file,
>  pid_t
>  fork_inferior (const char *exec_file_arg, const std::string &allargs,
>  	       char **env, void (*traceme_fun) (),
> -	       void (*init_trace_fun) (int), void (*pre_trace_fun) (),
> +	       gdb::function_view<void(int)> init_trace_fun,

There should be a space in "void (int)".

Thanks,
Pedro Alves