Re: [PATCH 3/7] ptrace_init_task: cleanup the usage of ptrace_link()
* Oleg Nesterov o...@redhat.com [2009-10-26 04:28:46]:
@@ -169,9 +164,9 @@ static inline void ptrace_init_task(stru
INIT_LIST_HEAD(child-ptraced);
child-parent = child-real_parent;
child-ptrace = 0;
- if (unlikely(ptrace)) {
+ if (unlikely(ptrace) (current-ptrace PT_PTRACED)) {
Any reason for directly accessing current-ptrace instead of accessing
thro task_ptrace(current) ?
Also would something like this suffice.
+ if (unlikely(ptrace) (task_ptrace(current)) {
i.e the first patch in this series removes checks to see if ptrace field
is set with PT_TRACED.
task_ptrace() != 0 if and only if PT_PTRACED bit is set, kill
some PT_PTRACED checks in tracehook.h.
Can you please explain how this situation is different from the
situation in tracehook.h?
child-ptrace = current-ptrace;
--
Thanks and Regards
Srikar
Télécom PRO tout en un : 29 Euros
Title: IC telecom Si ce message ne s'affiche pas correctement, visualisez la version en ligne La réduction des coûts généraux est un enjeu crucial pour la compétitivité de votre entreprise. Avec plus de 10 ans d'expérience dans les télécommunications, ICtelecom spécialiste de la voix sur IP, est l'inventeur du guichet unique pour les entreprises. Fort de sa maîtrise des technologies en télécommunications et des réseaux IP et bénéficiant dune position sans concurrence de premier plan sur le marché de la convergence (téléphonie fixe, téléphonie mobile, internet), ICtelecom développe pour le marché professionel des offres triple play packagées (voix, data, mobile). ICtelecom fournit plus de 12000 utilisateurs à travers plus de 2000 sites installés. SOYEZ DANS LES 50 PREMIERS* à souscrire à l'offre ICtelecom et recevez GRATUITEMENT ce caméscope sanyo *Offre soumise à conditions, valable pour toute souscription jusquau 31 décembre 2009 inclus, à partir de 3 lignes téléphoniques sous réserve dacceptation du dossier par ICtelecom. Remplissez ce formulaire pour ne plus recevoir notre newsletter. A rception, celui-ci sera pris en compte conformment la loi n 78-17 du 06-01-1978 relative linformatique, aux fichiers et aux liberts modifie par la loi n2004-801 du 06-08-2004
Re: [PATCH 118] (upstream) introduce kernel/ptrace.h
I'm skeptical this is the desireable way to move the code around. Of course, for all such things, I am fine with whatever upstream likes. But here are my concerns: That is not friendly to git history at all. If you move big chunks of code to different files, it's ideal to do it in a sequence of changes wherein the git file rename detection will grok what you are doing. With big cut and paste changes like this, it winds up looking like you introduced all the moved old code today. It takes manual looking and poking with git to see you moved it out of another file in this commit and then git annotate this~1 -- kernel/ptrace.c to find the commits that are really responsible for each line of code. I don't see how this route is any better than having the new and old code in kernel/ptrace.c with #ifdef. Conversely, just leave the common code in ptrace.c and compile it there. AFAICT the only static function that would need to be made global for that is __ptrace_detach. Thanks, Roland
utrace_control(,,UTRACE_SINGLESTEP)
[I have no idea why you appended this to that message introducing patches that are not related to this at all. Please use separate threads for separate topics, and choose clearly apropos Subject: lines.] I am thinking how can we fix utrace_control(SINGLESTEP). I don't have good ideas so far. But, perhaps we can add utrace-please_enable_single_step:1 ? If anything like this, it would be an enum utrace_resume_action field. Otherwise you are changing the API fundamentally. utrace_stop() and utrace_finish_jctl() can check this flag after wakeup What you mean here is an API where utrace_control's choice of resume_action takes effect without further callbacks when nothing else has requested any. As always, I want a discussion of the API semantics to be clear and separate from implementation details. Or, we can use ENGINE_SINGLESTEP, which probably makes more sense. Like ENGINE_STOP, it lives both in engine-flags and -utrace_flags. This takes us back to the old old utrace API where we had sticky state bits for the things that are now represented in utrace_resume_action. That is a very bad API choice IMHO, and its inherent unpleasantnesses are why we abandoned it before. I no longer think utrace_control() should just turn SINGLESTEP into REPORT silently. IMHO this characterization misrepresents what the API is today, and glosses over the real complexities that are why it is that way. There is no silent about it. If things are going on that affect what you asked for, then you get a callback to decide what you really want. The inability to react to other engines' effects this way was one of the major failings of the old sticky action bits API. The other thing that was completely wrong about sticky action state bits for single-step and block-step is that they are inherently just not sticky states. Each is a one-shot action that leads to a single provoked event (at most). It just doesn't make sense to represent them as states. Thanks, Roland
Re: [PATCH 118] (upstream) introduce kernel/ptrace.h
On 10/27, Roland McGrath wrote:
I'm skeptical this is the desireable way to move the code around.
Of course, for all such things, I am fine with whatever upstream likes.
But here are my concerns:
I am not sure this is the best choice too.
That is not friendly to git history at all.
Yes, I agree completely.
If you move big chunks of code
to different files, it's ideal to do it in a sequence of changes wherein
the git file rename detection will grok what you are doing.
Not sure I understand. Do you mean it is possible to move the code from
the old file to the new one in a git-friendly manner? Afaics, there is no
way to do this, git can only hanlde renames. (but my git skills is close
to zero).
I don't see how this route is any better than having the new and old code
in kernel/ptrace.c with #ifdef.
This is what I'd like to avoid as much as possible. As usual, this is very
subjective, but imho this way it will be very difficult to read the code,
even if we have a single ifdef which separates the old and new implementation.
Say, not good to have 2 almost identical ptrace_attach() implementations in
the same file.
Instead of the single ifdef, we can add a lot of them. For example, as for
ptrace_attach() we can do
int ptrace_attach(struct task_struct *task)
{
int retval;
audit_ptrace(task);
retval = -EPERM;
if (unlikely(task-flags PF_KTHREAD))
goto out;
if (same_thread_group(task, current))
goto out;
/*
* Protect exec's credential calculations against our
interference;
* interference; SUID, SGID and LSM creds get determined
differently
* under ptrace.
*/
retval = -ERESTARTNOINTR;
if (mutex_lock_interruptible(task-cred_guard_mutex))
goto out;
task_lock(task);
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH);
task_unlock(task);
if (retval)
goto unlock_creds;
#ifdef CONFIG_UTRACE
retval = ptrace_attach_task(task, 0);
if (unlikely(retval))
goto unlock_creds;
#endif
write_lock_irq(tasklist_lock);
retval = -EPERM;
if (unlikely(task-exit_state))
goto unlock_tasklist;
#ifdef CONFIG_UTRACE
BUG_ON(task-ptrace);
task-ptrace = PT_UTRACED;
#else
if (task-ptrace)
goto unlock_tasklist;
task-ptrace = PT_PTRACED;
#endif
if (capable(CAP_SYS_PTRACE))
task-ptrace |= PT_PTRACE_CAP;
__ptrace_link(task, current);
send_sig_info(SIGSTOP, SEND_SIG_FORCED, task);
retval = 0;
unlock_tasklist:
write_unlock_irq(tasklist_lock);
unlock_creds:
mutex_unlock(task-cred_guard_mutex);
out:
return retval;
}
but this is much, much worse and guess you didn't mean this.
Conversely, just leave the common code in
ptrace.c and compile it there. AFAICT the only static function that would
need to be made global for that is __ptrace_detach.
OK, agreed, this can work too.
In this case kernelptrace.c becomes
... the context of kernel/ptrace-common.h ...
#ifndef CONFIG_UTRACE
... other code ...
#endif
and we don't need CONFIG_PTRACE_OLD.
Do you agree with approach?
Oleg.
Re: utrace_control(,,UTRACE_SINGLESTEP)
On 10/27, Roland McGrath wrote:
[I have no idea why you appended this to that message introducing patches
that are not related to this at all. Please use separate threads for
separate topics, and choose clearly apropos Subject: lines.]
OK.
I am thinking how can we fix utrace_control(SINGLESTEP). I don't
have good ideas so far. But, perhaps we can add
utrace-please_enable_single_step:1 ?
If anything like this, it would be an enum utrace_resume_action field.
Otherwise you are changing the API fundamentally.
Not sure I understand... This is like utrace-vfork_stop:1, it is
only visible to utrace code.
utrace_stop() and utrace_finish_jctl() can check this flag after wakeup
What you mean here is an API where utrace_control's choice of resume_action
takes effect without further callbacks when nothing else has requested any.
As always, I want a discussion of the API semantics to be clear and
separate from implementation details.
Yes. please see below.
Or, we can use ENGINE_SINGLESTEP, which probably makes more sense.
Like ENGINE_STOP, it lives both in engine-flags and -utrace_flags.
This takes us back to the old old utrace API where we had sticky state
bits for the things that are now represented in utrace_resume_action.
That is a very bad API choice IMHO, and its inherent unpleasantnesses
are why we abandoned it before.
Agreed. I tried to think about ENGINE_SINGLESTEP more and came to the
same conclusion.
So. I am still not sure this is the best solution (in fact this doesn't
even look like a good solution to me), but afaics we can preserve the
current API and fix the problem if we add utrace-set_singlestep and
utrace-set_blockstep.
utrace_control()
case UTRACE_RESUME:
if (likely(reset)) {
user_disable_single_step(target);
utrace-set_singlestep = utrace-set_blockstep
= 0;
}
break;
case UTRACE_SINGLESTEP:
if (likely(reset))
utrace-set_singlestep = true;
else
ret = -EAGAIN;
break;
void utrace_finish_stop(...)
{
if (!(utrace-stop || utrace-set_singlestep))
return;
set_singlestep = utrace-set_singlestep;
spin_lock(utrace-lock);
utrace-stopped = trace-set_singlestep = false;
spin_unlock(utrace-lock);
if (set_singlestep)
user_enable_single_step(current);
}
utrace_finish_jctl() and utrace_stop() should call this new helper
instead of if (utrace-stopped) {} code.
I no longer think utrace_control() should just turn SINGLESTEP into
REPORT silently.
IMHO this characterization misrepresents what the API is today, and
glosses over the real complexities that are why it is that way.
There is no silent about it. If things are going on that affect what
you asked for, then you get a callback to decide what you really want.
The inability to react to other engines' effects this way was one of the
major failings of the old sticky action bits API.
Yes I see. And I agree, the current behaviour of utrace_control(SINGLESTEP)
which -set_blockstep hack tries to preserve is not very good.
As for ptrace. If utrace_control(SINGLESTEP) doesn't set TIF_SINGLESTEP,
then we need more (probably nasty) changes. report_quiesce/interrupt should
somehow figure out whether we need send_sigtrap() if -resume == XXXSTEP.
So. What do you think we should do for V1 ?
Oleg.
Re: [PATCH 0/7] utrace-ptrace V1
5/7 belongs first and I've already merged it as prerequisite to utrace. We can send that upstream without delay. I hope it can get queued quickly regardless of the review delays for the utrace and ptrace work. All the other preparatory patches are just to introduce PT_PTRACED as the distinction between the obsolete hooks for old ptrace and the remaining ptrace-specific kludges (unsafe_exec, tracer_task, and the interference with SIGCHLD/wait semantics). IMHO it's pretty questionable to do that rather than test those statically such that under CONFIG_UTRACE the old hooks are compiled away entirely (either via #ifdef or via things that reduce to if (0)). But moreover, this is fritter in the details of coexistence with the old implementation or sequencing of phasing it out. I really have no idea what the acceptable path for that is going to be at all. In the past, upstream reactions have ranged from utrace never! to no options, have only the utrace-based ptrace exist at all. I don't know that anyone is positively in favor of conditionally having two ptrace implementations, except perhaps as a compromise position for those who would prefer us to jump in the lake and never propose utrace again. I'm not at all sure that there isn't any one of the people with de facto veto power who will be dead-set against ever having both in the source at the same time. I don't think we can answer that except in the actual upstream review. So if this is v1 for upstream review, then take this path or whatever other makes for the necessary fritter being easiest to read (which is usually perceived upstream to mean least patch text) and get on with it. Thanks, Roland
Re: [PATCH 118] (upstream) introduce kernel/ptrace.h
Not sure I understand. Do you mean it is possible to move the code from the old file to the new one in a git-friendly manner? Afaics, there is no way to do this, git can only hanlde renames. (but my git skills is close to zero). What I meant is a sequence of patches like: 1. move non-common code from ptrace.c to ptrace-old.c 2. rename ptrace.c to ptrace.h 3. rename ptrace-old.c to ptrace.c or numerous other sequences that have that property, where one of the steps in the sequence renames a file without changing it. (That example is not a particularly good idea.) This is what I'd like to avoid as much as possible. As usual, this is very subjective, but imho this way it will be very difficult to read the code, even if we have a single ifdef which separates the old and new implementation. Say, not good to have 2 almost identical ptrace_attach() implementations in the same file. I agree that's ugly. Instead of the single ifdef, we can add a lot of them. For example, as for ptrace_attach() we can do Opinions also vary on whether that is more or less ugly. For some reason there seems to be a preference upstream for repeating identical declarations on both sides of an #ifdef over putting an #ifdef inside a function. I don't really get it. In this case kernelptrace.c becomes ... the context of kernel/ptrace-common.h ... #ifndef CONFIG_UTRACE ... other code ... #endif and we don't need CONFIG_PTRACE_OLD. Do you agree with approach? That's what I had in mind. But I'm almost sorry I raised the issue. I don't think there is much benefit to debating these details amongst ourselves. The right answer is whatever the upstream reviewers demand. You and I just want something merged ASAP and all the nits are secondary. Thanks, Roland
Re: utrace_control(,,UTRACE_SINGLESTEP)
On 10/28, Oleg Nesterov wrote: As for ptrace. If utrace_control(SINGLESTEP) doesn't set TIF_SINGLESTEP, then we need more (probably nasty) changes. report_quiesce/interrupt should somehow figure out whether we need send_sigtrap() if -resume == XXXSTEP. Or. We can add the hack below for V1, then fix/cleanup this. Oleg. --- a/kernel/ptrace-utrace.c +++ b/kernel/ptrace-utrace.c @@ -227,6 +227,14 @@ static void ptrace_wake_up(struct task_s } } + // XXX!!! temporary hack. + // this and ptrace_resume()-send_sigtrap() + // should not exist !!! + if (action = UTRACE_SINGLESTEP) + user_enable_single_step(tracee); + else if (action = UTRACE_BLOCKSTEP) + user_enable_block_step(tracee); + if (action != UTRACE_REPORT) ptrace_context(engine)-stop_code = 0; utrace_control(tracee, engine, action);
Re: [PATCH 118] (upstream) introduce kernel/ptrace.h
On 10/27, Roland McGrath wrote: In this case kernelptrace.c becomes ... the context of kernel/ptrace-common.h ... #ifndef CONFIG_UTRACE ... other code ... #endif and we don't need CONFIG_PTRACE_OLD. Do you agree with approach? That's what I had in mind. OK, will do. The right answer is whatever the upstream reviewers demand. Which we can't predict. But your suggestion looks more simple and clean. Especially because we can avoid CONFIG_PTRACE_OLD. You and I just want something merged ASAP and all the nits are secondary. Yes ;) Oleg.
Re: utrace_control(,,UTRACE_SINGLESTEP)
Not sure I understand... This is like utrace-vfork_stop:1, it is only visible to utrace code. Show me the change the the utrace_control kerneldoc wording that makes it match what difference you propose this implementation detail would make. When you consider other engines using UTRACE_BLOCKSTEP and all other such interactions, I think you'll find that there isn't a clean API description that could be implemented using a flag like you suggested. So. I am still not sure this is the best solution (in fact this doesn't even look like a good solution to me), but afaics we can preserve the current API and fix the problem if we add utrace-set_singlestep and utrace-set_blockstep. This approach loses the existing possibility e.g. for an engine that tried UTRACE_BLOCKSTEP to find out that an earlier engine is already using UTRACE_SINGLESTEP and so the stop it gets next won't necessarily be at a block boundary. As I alluded to before, the version of this approach that seems like it could be clean is to store an enum utrace_resume_action rather than a flag (or two flags). Thinking about it more, my intuition is that this could be the way to go. This field could also replace the report and interrupt bits we have now, and that starts to feel clean. In fact, with a broader view it might not even be desireable to do user_enable_single_step in the tracer rather than the tracee. I always thought of it as desireable because it is how ptrace does it now, and I recalled that on one arch (s390) enabling single-step has to fiddle CPU state that is normally set by context switch--so on that machine, it has to do an extra state-fiddle step to enable in self after context switch. But, on at least as many machines (x86, score) it could be advantageous to enable in self rather than enable from tracer. (On all the other machines, it really doesn't matter--it's just purely in register bits like you would expect.) Those two use access_process_vm to look at the PC instruction, and we could (later) optimize those to use copy_from_user when called on current, which has much lower overhead. So I now have reversed in complementary directions. As to the API, I do think it would be nice to avoid always getting a callback when there really is nothing else going on--though I still insist engines are broken if they don't have one so they can react to other engines' actions. But as to the mechanics, I now think we should favor calling user_enable_single_step et al in current rather than not. As for ptrace. If utrace_control(SINGLESTEP) doesn't set TIF_SINGLESTEP, then we need more (probably nasty) changes. report_quiesce/interrupt should somehow figure out whether we need send_sigtrap() if -resume == XXXSTEP. I may have lost the plot again, sorry. You're talking about the case of PTRACE_SINGLESTEP at a syscall-exit stop, which is not even consistent across machines in the vanilla kernel. Right? Thanks, Roland
Re: [PATCH 0/7] utrace-ptrace V1
On 10/27, Roland McGrath wrote: 5/7 belongs first and I've already merged it as prerequisite to utrace. We can send that upstream without delay. I hope it can get queued quickly regardless of the review delays for the utrace and ptrace work. Agreed, I'll send it to Andrew. All the other preparatory patches are just to introduce PT_PTRACED as the distinction between the obsolete hooks for old ptrace and the remaining ptrace-specific kludges (unsafe_exec, tracer_task, and the interference with SIGCHLD/wait semantics). Yes. And, although you didn't say this, I completely agree: this is dirty hack. IMHO it's pretty questionable to do that rather than test those statically such that under CONFIG_UTRACE the old hooks are compiled away entirely (either via #ifdef or via things that reduce to if (0)). Agreed! Hopefully we can do this later. As you understand, the goal is to make the first series as small as possible, where small means the number of changes outside of ptrace.c. But moreover, this is fritter in the details of coexistence with the old implementation or sequencing of phasing it out. I really have no idea what the acceptable path for that is going to be at all. In the past, upstream reactions have ranged from utrace never! to no options, have only the utrace-based ptrace exist at all. Yes. CONFIG_UTRACE should go away, but when this will happen? We have to fix !HAVE_ARCH_TRACEHOOK arches first and to ensure bobody in arch/ plays with ptrace internals. I don't know that anyone is positively in favor of conditionally having two ptrace implementations, At least, we don't have CONFIG_UTRACE_PTRACE. I don't think we can answer that except in the actual upstream review. Yep. So if this is v1 for upstream review, Yes, I hope so. Oleg.
Re: utrace_control(,,UTRACE_SINGLESTEP)
On 10/27, Roland McGrath wrote: Not sure I understand... This is like utrace-vfork_stop:1, it is only visible to utrace code. Show me the change the the utrace_control kerneldoc wording that makes it match what difference you propose this implementation detail would make. When you consider other engines using UTRACE_BLOCKSTEP and all other such interactions, I think you'll find that there isn't a clean API description that could be implemented using a flag like you suggested. Again, I don't understand. Let me repeat, I tried to propose something that do not make any visible difference except fixes the problem with get_user_pages() under spin_lock(). And yes, while I don't pretend I know what should be done, I agree the current API (I don't mean the documentation, I mean the actual code) is not right. So. I am still not sure this is the best solution (in fact this doesn't even look like a good solution to me), but afaics we can preserve the current API and fix the problem if we add utrace-set_singlestep and utrace-set_blockstep. This approach loses the existing possibility e.g. for an engine that tried UTRACE_BLOCKSTEP to find out that an earlier engine is already using UTRACE_SINGLESTEP and so the stop it gets next won't necessarily be at a block boundary. Afaics, the current code can't work properly in this case anyway. But yes, the pseudo-code I showed is not complete, it was only to roughly explain what I mean. As I alluded to before, the version of this approach that seems like it could be clean is to store an enum utrace_resume_action rather than a flag (or two flags). Thinking about it more, my intuition is that this could be the way to go. This field could also replace the report and interrupt bits we have now, and that starts to feel clean. At firts glance, this looks right to me. In fact, with a broader view it might not even be desireable to do user_enable_single_step in the tracer rather than the tracee. I agree. But I can't explain why I agree ;) I mean, I feel this should be more clean, but given that I do not understand the low level details even remotely I can't judge. Those two use access_process_vm to look at the PC instruction, and we could (later) optimize those to use copy_from_user when called on current, which has much lower overhead. Yes, I thought about this too. So I now have reversed in complementary directions. As to the API, I do think it would be nice to avoid always getting a callback when there really is nothing else going on--though I still insist engines are broken if they don't have one so they can react to other engines' actions. But as to the mechanics, I now think we should favor calling user_enable_single_step et al in current rather than not. OK. Unless I misunderstand avoid always getting a callback above, this means the tracee should call enable_step() after wakeup in utrace_stop(), yes? In this case ptrace doesn't need any changes for now. As for ptrace. If utrace_control(SINGLESTEP) doesn't set TIF_SINGLESTEP, then we need more (probably nasty) changes. report_quiesce/interrupt should somehow figure out whether we need send_sigtrap() if -resume == XXXSTEP. I may have lost the plot again, sorry. You're talking about the case of PTRACE_SINGLESTEP at a syscall-exit stop, which is not even consistent across machines in the vanilla kernel. Right? Not necessary in syscall-exit stop, but yes. And yes, this is not even consistent across machines (as you explained before), that is why I am very nervous when I think about the changes in this area. Oleg.
Re: [PATCH 6/7] introduce kernel/ptrace.h
On 10/27, Ananth N Mavinakayanahalli wrote: On Mon, Oct 26, 2009 at 04:28:52AM +0100, Oleg Nesterov wrote: No functional changes, preparation for utrace-ptrace. Introduce kernel/ptrace.h and move the code which can be shared with the new implementation into this new header. Did you want this to be a .h? Couldn't this just be a ptrace-common.c whose functions can be called from ptrace.c, while a ptrace-common.h can have the function definitions? I think Roland has a better idea, hopefully you read this discussion. Oleg.
Re: [PATCH 7/7] implement utrace-ptrace
--- /dev/null 2009-10-25 19:46:00.608018007 +0100
+++ V1/kernel/ptrace-utrace.c 2009-10-26 03:56:46.0 +0100
Generally, needs more comments. That's not news, I'm sure.
But just giving reactions as I would if doing upstream review
without other context.
+struct ptrace_context {
+ int options;
+
+ int signr;
+ siginfo_t *siginfo;
+
+ int stop_code;
+ unsigned long eventmsg;
+
+ enum utrace_resume_action resume;
+};
If you are lining up whitespace in field decls, do them all. This struct
merits a top comment about what it is and where it hangs. Either in more
top comment or interspersed, at least a little on what the fields are for.
+#define PT_UTRACED 0x1000
+
+#define PTRACE_O_SYSEMU 0x100
+
+#define PTRACE_EVENT_SYSCALL_ENTRY (1 16)
+#define PTRACE_EVENT_SYSCALL_EXIT(2 16)
+#define PTRACE_EVENT_SIGTRAP (3 16)
+#define PTRACE_EVENT_SIGNAL (4 16)
+/* events visible to user-space */
+#define PTRACE_EVENT_MASK0x
Needs comments about what it is, and why it is not in linux/ptrace.h.
Personally I am glad to have these private bits in private source only.
But it certainly looks to the casual observer like these belong next to
the macros for the adjacent bits.
+static inline bool ptrace_event_pending(struct ptrace_context *context)
s/context/ctx/ throughout for variable names (but leave ptrace_context as
the struct name).
+ if (action != UTRACE_REPORT)
+ ptrace_context(engine)-stop_code = 0;
+ utrace_control(tracee, engine, action);
__must_check
+/**
+ * ptrace_traceme -- helper for PTRACE_TRACEME
Don't use /** kerneldoc magic. You're not matching the whole form,
and these are not functions we extract docs for anyway.
+void ptrace_notify_stop(struct task_struct *tracee)
+{
+ struct utrace_engine *engine = ptrace_lookup_engine(tracee);
+
+ if (IS_ERR(engine)) {
+ // XXX: temporary check, wrong with mutlitracing
+ WARN_ON(tracee-state != TASK_RUNNING);
+ return;
Explain this scenario at least a little in the comment.
+#ifdef PTRACE_SINGLESTEP
+ case PTRACE_SINGLESTEP:
+#endif
+#ifdef PTRACE_SINGLEBLOCK
+ case PTRACE_SINGLEBLOCK:
+#endif
+#ifdef PTRACE_SYSEMU
+ case PTRACE_SYSEMU:
+ case PTRACE_SYSEMU_SINGLESTEP:
+#endif
+ case PTRACE_SYSCALL:
+ case PTRACE_CONT:
+ ret = ptrace_resume(child, engine, request, data);
+ break;
Since ptrace_resume_action has a switch with these #ifdef's anyway,
maybe make this the default case here and give that other switch a
default error case.
+ case PTRACE_KILL:
+ ret = 0;
+ if (!child-exit_state) /* already dead */
+ ret = ptrace_resume(child, engine, request, SIGKILL);
+ break;
This could be inside ptrace_resume too. That fits better if you fold
ptrace_resume_action into ptrace_resume, and I don't see any reason at
all not to do that anyway.
Thanks,
Roland
