lkml.org 
[lkml]   [2009]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 1/14] x86, ptrace: add arch_ptrace_report_exit
    On 03/27, Metzger, Markus T wrote:
    >
    > >-----Original Message-----
    > >From: Oleg Nesterov [mailto:oleg@redhat.com]
    > >
    > >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.



    \
     
     \ /
      Last update: 2009-03-27 18:15    [W:0.026 / U:30.796 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site