Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
8. Completely off-topic, but utrace_control() has a very strange comment under case UTRACE_INTERRUPT, * When it's stopped, we know it's always going * through utrace_get_signal() and will recalculate. can't recall if it were ever true, but surely it is not now? I

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
4. One of the changes in utrace_get_signal() doesn't look exactly right. if (utrace-resume UTRACE_RESUME || utrace-signal_handler) { ... if (resume UTRACE_REPORT) { report.action = resume;

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
7. utrace_attach_task() has an implicit wmb() between -utrace = new_utrace and -utrace_flags = REAP, this is good. But, for example, tracehook_force_sigpending() does not have rmb(), this means utrace_interrupt_pending() can OOPS in theory. Ok. Please send a patch. Off hand it

Re: [PATCH 0/1] Was: utrace-cleanup branch

2009-11-03 Thread Roland McGrath
2. Cosmetic, but the usage of utrace_task_alloc() looks a bit strange. Why it returns bool, not struct utrace * ? The pointer it would return is always target-utrace or it's NULL. So the bool just says which of those it would be instead. Either way I imagine it should be inlined so the