Re: [RFC] Kill pthread_ops_hack

[Vladimir Prus <vladimir@codesourcery.com>]

On Friday 15 August 2008 17:42:17 Mark Kettenis wrote:
> > 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.

Only in the text of the message :-)

> > 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, ...);
> }

OK.

- Volodya