Re: [PATCH] Improve ptrace-error detection on Linux targets

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Monday, August 19 2019, Pedro Alves wrote:

> Two comments on implementation, that I think apply 
> equally to the new version you're working on as well:

Thanks for the review.  I didn't reply before, but I used your
suggestions in the next version.

> #1 - A target method (target_ptrace_me_fail_reason) looks like the
>   wrong place to put this, to me.  As I understand it, the core of
>   gdb isn't ever calling the target_ptrace_me_fail_reason.  This is
>   all code that is implementation detail of the Linux target
>   implementation.
>
>   I'd prefer some other way to address this.  An idea would be to have
>   a function pointer in inf-ptrace.c that defaults to an implementation
>   that returns the empty string:
>
>    // in inf-ptrace.c
>
>    static std::string
>    default_inf_ptrace_me_fail_reason (int err)
>    {
>      return {};
>    }
>
>    std::string (*inf_ptrace_me_fail_reason) (int err) = default_inf_ptrace_me_fail_reason;
>
>    // in inf-ptrace.h
>
>    extern std::string (*inf_ptrace_me_fail_reason) (int err);
>
>    And then in linux-nat.c:_initialize_linux_nat, you'd override the
>    implementation, like:
>
>    std::string
>    linux_ptrace_me_fail_reason (int err)
>    {
>      ...
>    }
>
>    void
>    _initialize_linux_nat ()
>    {
>       ... 
>       inf_ptrace_me_fail_reason = linux_ptrace_me_fail_reason;
>    }
>
>
>   Note I'm not suggesting anything with virtual methods before
>   of the next point.
>
> #2 - What about gdbserver?  Don't we want the same nice error
> messages in gdbserver?
>
> See gdbserver/linux-low.c:linux_ptrace_fun.
>
> This would suggest putting that linux_ptrace_me_fail_reason
> function in gdb/nat/ so that it would be used by gdbserver too.

Thanks.  I followed both of your suggestions, and will submit a v2 soon.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/