This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Kill pthread_ops_hack
- From: Mark Kettenis <mark dot kettenis at xs4all dot nl>
- To: vladimir at codesourcery dot com
- Cc: gdb-patches at sources dot redhat dot com
- Date: Fri, 15 Aug 2008 15:42:17 +0200 (CEST)
- Subject: Re: [RFC] Kill pthread_ops_hack
- References: <200808151715.55875.vladimir@codesourcery.com>
> 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, ...);
}