Re: [PATCH v4 2/9] add "this" pointers to more target APIs

Tom Tromey
2013-10-28T16:37:23Z

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 10/22/2013 06:59 PM, Tom Tromey wrote:
>> @@ -4758,8 +4756,8 @@ static void
>> linux_nat_close (void)
>> {
>> /* Unregister from the event loop. */
>> - if (linux_nat_is_async_p ())
>> - linux_nat_async (NULL, 0);
>> + if (linux_nat_is_async_p (linux_ops))
>> + linux_nat_async (linux_ops, NULL, 0);

Pedro> It doesn't matter much since linux_nat_async doesn't use it's
Pedro> argument, but FYI, this one here doesn't look 100% correct.
Pedro> "linux_ops" is the single-threaded target linux-nat itself
Pedro> delegates some work to (it's never pushed anywhere, etc.).
Pedro> Ideally, to_close would be passed in a "this" pointer too.

The multi-target branch has a more invasive transformation of the target
API. It doesn't add a this pointer to to_close, but it does implement a
target_ops / target object split, and it changes more targets to use
to_xclose, which does take a "this".

In this particular spot, I don't remember why I used linux_ops.

It is all moot, I think. There is no reason for linux-nat to ever call
linux_nat_is_async_p any more. I think we can drop all the dead code
instead. I noted this in the first submission and said I will do it in
a followup; but I think I'll just tack it on to this series instead.

>> static int
>> -record_full_stopped_by_watchpoint (void)
>> +record_full_stopped_by_watchpoint (struct target_ops *ops)
>> {
>> if (RECORD_FULL_IS_REPLAY)
>> return record_full_hw_watchpoint;
>> else
>> - return record_full_beneath_to_stopped_by_watchpoint ();
>> + return record_full_beneath_to_stopped_by_watchpoint (find_target_beneath (ops));
>> }
>>

Pedro> Line too long.

Fixed.

Tom