Re: [PATCH v2 0/2] Initial support for FreeBSD/aarch64

[John Baldwin <jhb at FreeBSD dot org>]

On 9/5/17 3:33 PM, Pedro Alves wrote:
> 
> On 09/05/2017 03:51 AM, John Baldwin wrote:
>> On 9/4/17 6:52 AM, Pedro Alves wrote:
>>> On 08/16/2017 02:57 PM, John Baldwin wrote:
>>>> This series adds initial support for FreeBSD/aarch64.  It does not
>>>> yet include support for hardware breakpoints/watchpoints as that is
>>>> not currently supported by FreeBSD's kernel.
>>>>
>>>> I realized yesterday that I had forgotton to 'git add' the
>>>> aarch64-fbsd-nat.c file for the second patch, so this series adds
>>>> the missing file.
>>>
>>> I skimmed this and it looks good to me.
>>>
>>> One tiny comment:
>>>
>>>  > +/* Provide a prototype to silence -Wmissing-prototypes.  */
>>>  > +extern initialize_file_ftype _initialize_aarch64_fbsd_tdep;
>>>
>>> ...
>>>
>>>  > +/* Provide a prototype to silence -Wmissing-prototypes.  */
>>>  > +void _initialize_aarch64_fbsd_nat (void);
>>>
>>> These prototypes no longer make sense in C++ [1], and we
>>> could/should remove all the existing ones to avoid confusion.
>>
>> I assumed it was more for the warning per the comment.
> 
> -Wmissing-prototypes doesn't exist in G++, only in 
> GCC-the-C-compiler, so the comments about the warning
> are all out of date now.
> 
> It doesn't exist in G++ because unlike in C, in C++ there's
> no such thing as a declaration that is not a prototype.

Ok.

>> I wonder if
>> we shouldn't think about replacing init.c with something more C++y
>> anyway?  We could perhaps do it incrementally by adding a global
>> container of function pointers and a wrapper class whose constructor
>> accepts a function pointer and adds it to the container.  Then we
>> would do a for-each over the container at the same place as
>> initialize_all_files().  If the ordering doesn't matter then this
>> could be rolled out over time (i.e. adding the new container and
>> wrapper class and then individual _init* functions could be converted
>> to the new scheme).
> 
> Not sure.  OTOH, that seems like it'd add a little runtime/startup time
> overhead for no real gain?  For most cases where the ordering truly doesn't
> matter, we could make the wrapper classes's ctor call the function
> directly, though I wonder whether that's a good direction to go if
> we consider moving gdb in the direction of making it a dlopen-able library.

That's fair, it was more about using the language in place of a build-time
"hack".  Regardless, I do think you would want to probably not call the
function directly from the constructor since 'initialize_all_files()' now is
called much later than when traditional .init/.ctors are called.  Perhaps if
some other objects that existing _initialize_foo() used were changed to be
singletons you could take that approach.  init.c is probably good enough to
not spend more effort trying to replace it.

-- 
John Baldwin