From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: vladimir@codesourcery.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFC] Kill pthread_ops_hack
Date: Fri, 15 Aug 2008 13:45:00 -0000 [thread overview]
Message-ID: <200808151342.m7FDgHru018919@brahms.sibelius.xs4all.nl> (raw)
In-Reply-To: <200808151715.55875.vladimir@codesourcery.com> (message from Vladimir Prus on Fri, 15 Aug 2008 17:15:55 +0400)
> 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, ...);
}
next prev parent reply other threads:[~2008-08-15 13:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-15 13:16 Vladimir Prus
2008-08-15 13:45 ` Mark Kettenis [this message]
2008-08-15 14:00 ` Vladimir Prus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200808151342.m7FDgHru018919@brahms.sibelius.xs4all.nl \
--to=mark.kettenis@xs4all.nl \
--cc=gdb-patches@sources.redhat.com \
--cc=vladimir@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox