This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Kill pthread_ops_hack


> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 15 Aug 2008 17:15:55 +0400
> 
> 
> Whenever a target that uses ptrace is created, GDB calls
> inf_ptrace_target, and then augments the result with whatever
> methods are necessary for the current OS, CPU, etc. However, when
> inf_ptrace_target is called, the result is stored in a global
> variable ptrace_ops_hack, which is then used by inf-ptrace.c in a
> few places. Of course, having a variable named whatever_hack in GDB
> codebase is already bad, but this design also means that we have
> have only one pthread-based target active at a time, which does not
> seem like a good thing.
> 
> In fact, pthread_ops_hack is a consequence of current design of
> target stack.  When we do 'run', the linux target is not pushed yet,
> and find_default_create_inferior looks for a target, and calls its
> to_create_inferior method. As soon as we create inferiour, we need
> to push the target on stack, so that further operations will apply
> to now-existing inferiour. But to_create_inferior is not passed the
> struct target_ops pointer, so it does not know what to push. This
> patch makes to_create_inferiour and few other methods, take struct
> target_ops pointer, and kills pthread_ops_hack.

Looks like you're confusing ptrace and pthreads here.

> I have only converted few targets -- linux and remote. Converting
> others will be a mechanical task for adding a parameter to function,
> but before I go on with that -- anybody has objections to the
> general direction of this patch?

No this is the obvious solution.  However:

>  static void
> -inf_ptrace_him (int pid)
> +inf_ptrace_create_inferior (struct target_ops *ops,
> +			    char *exec_file, char *allargs, char **env,
> +			    int from_tty)
>  {
> -  push_target (ptrace_ops_hack);
> +  int pid = fork_inferior (exec_file, allargs, env, inf_ptrace_me, NULL,
> +			   NULL, NULL);

Could you please not write code like that?  The stuff fork_inferior()
does goes way beyond what's necessary to initialize pid.  Better write
it like:

{
  int pid;

  pid = fork_inferior(exec_file, ...);
}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]