Re: [patch 1/14] x86, ptrace: add arch_ptrace_report_exit

From: Oleg Nesterov
Date: Fri Mar 27 2009 - 13:11:35 EST


On 03/27, Metzger, Markus T wrote:
>
> >-----Original Message-----
> >From: Oleg Nesterov [mailto:oleg@xxxxxxxxxx]
> >
> >This needs Rolan'd review.
> >
> >But I'd say this has nothing to do with tracehooks. And why do
> >you pass *exit_code to arch_ptrace_report_exit() ?
> >
> >Just add arch_ptrace_report_exit(void) into do_exit() ?
> >
> >From the 3/14 patch:
> >
> > #define arch_ptrace_report_exit(code) x86_ptrace_report_exit(code)
> >
> > void x86_ptrace_report_exit(long exit_code)
> > {
> > ptrace_bts_exit();
> > }
> >
> >This is a bit strange. Why do we need 2 functions, ptrace_bts_exit() and
> >x86_ptrace_report_exit() which just calls the first one?
>
> I did not want to take any shortcuts. I try to maintain the structure
> general_function()->ptrace_report()->arch_ptrace_report().

I see. And honestly, this doesn't look good to me. Yes, this is subjective.

Say, Regardless of CONFIG_X86_PTRACE_BTS we have the non-empty and non-inline
x86_ptrace_untrace() which just calls ptrace_bts_untrace(). And ptrace_bts_untrace()
depends on CONFIG_X86_PTRACE_BTS.

But this is minor.

> Recently, tracehook_report_whatever() calls were added which either do the
> ptrace work directly or call a ptrace function. I try to use those calls, where possible.

Up to Roland, but I still think tracehook_report_whatever() is not the
good place for this stuff. And tracehooks will be changed soon by utrace.

In any case I don't understand why you added yet another helper, you could
just add arch_ptrace_report_exit() into tracehook_report_exit().

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/