[pushed] Re: [PATCH 03/16 v2] Refactor ptrace extended event status

Breazeal, Don
2014-09-19T18:14:53Z

On 9/9/2014 4:31 AM, Pedro Alves wrote:
> Hi Don,
>
> Let's get this one out of the way.
>
> On 08/21/2014 01:29 AM, Don Breazeal wrote:
>> This patch implements functions for identifying and extracting extended
>> ptrace event information from a Linux wait status. These are just
>> convenience functions intended to hide the ">> 16" used to extract the
>> event from the wait status word, replacing the hard-coded shift with a more
>> descriptive function call. This is preparatory work for implementation of
>> follow-fork and detach-on-fork for extended-remote linux targets.
>>
>> The functions linux_is_extended_waitstatus and
>> linux_ptrace_get_extended_event are defined in nat/linux-ptrace.c, and
>> called in linux-nat.c and gdbserver/linux-low.c.
>>
>> My initial approach was to implement predicates for every extended event,
>> e.g. linux_is_traced_clone (status), linux_is_traced_fork (status), but
>> that didn't fit the current implementation as well, bloated the code a bit,
>> and didn't add anything to readability, so I went with just extracting the
>> event bits from the status instead.
>>
>> Tested on x64 Ubuntu Lucid, native only.
>
> This is OK. Please push.

Thanks for the review. This is pushed in now, with fixes detailed
below.

>
> Though I wonder why not push the SIGTRAP check to
> linux_is_extended_waitstatus too.

My thinking was to implement something at the same level as WIFSTOPPED
et al.

>
>> gdb/
>> 2014-08-20 Don Breazeal <donb@codesourcery.com>
>>
>> * linux-nat.c (linux_handle_extended_wait): Call
>> linux_ptrace_get_extended_event.
>> (wait_lwp): Call linux_is_extended_waitstatus.
>> (linux_nat_filter_event): Call linux_ptrace_get_extended_event
>> and linux_is_extended_waitstatus.
>> * nat/linux-ptrace.c (linux_test_for_tracefork): Call
>> linux_ptrace_get_extended_event.
>> (linux_ptrace_get_extended_event): New function.
>> (linux_is_extended_waitstatus): New function.
>> * nat/linux-ptrace.h: Declare new functions.
>
> Please spell out the new declarations. Like:
>
> * nat/linux-ptrace.h (linux_ptrace_get_extended_event)
> (linux_is_extended_waitstatus): New declarations.

Fixed.

>
>>
>> gdbserver/
>> 2014-08-20 Don Breazeal <donb@codesourcery.com>
>> * linux-low.c (handle_extended_wait): Call
>
> Missing empty line above.

Fixed.

>
>> linux_ptrace_get_extended_event.
>> (get_stop_pc, get_detach_signal, linux_low_filter_event): Call
>> linux_is_extended_waitstatus.
>
> Thanks,
> Pedro Alves
>