Re: [PATCHv2 6/7] hurd: add gnu_target pointer to fix thread API calls

[Simon Marchi]

On 2020-05-29 6:03 p.m., Samuel Thibault wrote:
> Fixes
> 
> ../../gdb/gnu-nat.c:1110:28: error: cannot convert ‘ptid_t’ to 
> ‘process_stratum_target*’
>  1110 |        thread_change_ptid (inferior_ptid, ptid);
> 
> and others related to 5b6d1e4fa ("Multi-target support")
> 
> gdb/ChangeLog:
> 
> 2020-05-29  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> 
>       * gnu-nat.h (gnu_target): New variable declaration.
>       * i386-gnu-nat.c (_initialize_i386gnu_nat): Initialize
>       gnu_target.
>       * gnu-nat.c (gnu_target): New variable.
>       (inf_validate_procs): Pass gnu_target to thread_change_ptid,
>       add_thread_silent, and add_thread calls.
>       (gnu_nat_target::create_inferior): Pass gnu_target to
>       add_thread_silent, thread_change_ptid call.
>       (gnu_nat_target::detach): Pass gnu_target to detach_inferior
>       call.
> 
> diff --git a/gdb/gnu-nat.h b/gdb/gnu-nat.h
> index 77c57817b2..7c36778394 100644
> --- a/gdb/gnu-nat.h
> +++ b/gdb/gnu-nat.h
> @@ -150,4 +150,7 @@ struct gnu_nat_target : public inf_child_target
>    void stop (ptid_t) override;
>  };
>  
> +/* The final/concrete instance.  */
> +extern gnu_nat_target *gnu_target;
> +
>  #endif /* GNU_NAT_H */
> diff --git a/gdb/i386-gnu-nat.c b/gdb/i386-gnu-nat.c
> index afbe8eff49..6382ca8acb 100644
> --- a/gdb/i386-gnu-nat.c
> +++ b/gdb/i386-gnu-nat.c
> @@ -439,6 +439,8 @@ _initialize_i386gnu_nat ()
>    x86_set_debug_register_length (4);
>  #endif /* i386_DEBUG_STATE */
>  
> +  gnu_target = &the_i386_gnu_nat_target;
> +
>    /* Register the target.  */
>    add_inf_child_target (&the_i386_gnu_nat_target);
>  }
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index 3b438a9a43..78e9ab7f71 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -84,6 +85,8 @@ extern "C"
>  #include "msg_U.h"
>  }
>  
> +struct gnu_nat_target *gnu_target;
> +
>  static process_t proc_server = MACH_PORT_NULL;
>  
>  /* If we've sent a proc_wait_request to the proc server, the pid of the
> @@ -1106,12 +1109,12 @@ inf_validate_procs (struct inf *inf)
>           if (inferior_ptid == ptid_t (inf->pid))
>             /* This is the first time we're hearing about thread
>                ids, after a fork-child.  */
> -           thread_change_ptid (inferior_ptid, ptid);
> +           thread_change_ptid (gnu_target, inferior_ptid, ptid);
>           else if (inf->pending_execs != 0)
>             /* This is a shell thread.  */
> -           add_thread_silent (ptid);
> +           add_thread_silent (gnu_target, ptid);
>           else
> -           add_thread (ptid);
> +           add_thread (gnu_target, ptid);
>         }
>        }
>  
> @@ -2149,7 +2152,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
>    /* We have something that executes now.  We'll be running through
>       the shell at this point (if startup-with-shell is true), but the
>       pid shouldn't change.  */
> -  add_thread_silent (ptid_t (pid));
> +  add_thread_silent (gnu_target, ptid_t (pid));
>  
>    /* Attach to the now stopped child, which is actually a shell...  */
>    inf_debug (inf, "attaching to child: %d", pid);
> @@ -2167,7 +2170,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
>    inf_resume (inf);
>  
>    /* We now have thread info.  */
> -  thread_change_ptid (inferior_ptid,
> +  thread_change_ptid (gnu_target, inferior_ptid,
>                     ptid_t (inf->pid, inf_pick_first_thread (), 0));
>  
>    gdb_startup_inferior (pid, START_INFERIOR_TRAPS_EXPECTED);
> @@ -2273,7 +2276,7 @@ gnu_nat_target::detach (inferior *inf, int from_tty)
>    inf_detach (gnu_current_inf);
>  
>    inferior_ptid = null_ptid;
> -  detach_inferior (find_inferior_pid (pid));
> +  detach_inferior (find_inferior_pid (gnu_target, pid));
>  
>    maybe_unpush_target ();
>  }
> 

I think it would have been more straightforward and clean to use 
`inf->process_target ()`
(assuming it's the correct thing to do), given that you have access to the 
appropriate
inferior everywhere you made changes.

But I also see that you've pretty much replicated what the linux-nat target 
does, so it
is ok as well if you want to keep it that way.

Simon