Re: [PATCH 31/40] target_ops/C++: Base FreeBSD target

[Pedro Alves <palves at redhat dot com>]

On 04/17/2018 05:05 PM, John Baldwin wrote:
> On Saturday, April 14, 2018 08:09:44 PM Pedro Alves wrote:
>> The
>>
>>   $architecture x NetBSD/OpenBSD/FreeBSD
>>
>> support matrix complicates things a bit.  There's common BSD target
>> code, and there's common architecture-specific code shared between the
>> different BSDs.  Current, all that is stiched together to form a final
>> target, via the i386bsd_target, x86bsd_target, fbsd_nat_add_target
>> functions etc.
>>
>> Introduces a fbsd_nat_target base/prototype target.  To be used in
>> following patches.
> 
> I will do some tests of FreeBSD/amd64 first and let you know what I find.

Thank you!

> One small thing I noticed:
> 
>> diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
>> index 8326b96db7..a4418bb8f8 100644
>> --- a/gdb/fbsd-nat.h
>> +++ b/gdb/fbsd-nat.h
>> +
>> +#ifdef PL_FLAG_EXEC
>> +  int insert_exec_catchpoint (int) override;
>> +  int remove_exec_catchpoint (int) override;
>> +#endif
>> +
>> +#ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
>> +  int set_syscall_catchpoint (int, bool, int, gdb::array_view<const int>)
>> +    override;
>> +#endif /* HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE */
>> +
>> +#endif /* PT_LWPINFO */
>> +};
> 
> Do we want to be consistent about whether or not to have comments for
> #endif's?  Most in this change don't but these two do.

I think I have added the one for PT_LWPINFO thinking that the
corresponding #ifdef is so far away, and then added the one just
above too, while missing the others.

I don't have a strong opinion on whether all should have the
comment, I'm fine either way since the #ifdef blocks are small,
i.e., removing the comment for HAVE_STRUCT_PTRACE_LWPINFO_PL_SYSCALL_CODE
is totally fine with me and may even reduce noise.  I do think
the comment for "#endif PT_LWPINFO" is beneficial, however.

> 
> (Also, my initial impression is that the #ifdef's are more readable in this
> version than the current code)
> 

Yeah, the #endif for PT_LWPINFO is the one that I think I
recall gave me pause in the current code.

Thanks,
Pedro Alves