Re: C++-compat clean build

[Ondrej Oprala <ooprala@redhat.com>]

On 10/01/2013 02:53 PM, Jan Kratochvil wrote:
> Hi Ondrej,
>
> On Tue, 01 Oct 2013 13:25:34 +0200, Ondrej Oprala wrote:
>> this is the first of a few patches I intend to write to make gdb
>> code compile cleanly with -Wc++-compat.
>> The idea is to make separate patches for respective subdirs under
>> gdb/, unless someone objects ofc.
> this is a too huge patch.  It should import first archer/tromey/c++ which is
> already separated into specific parts, that is each commit in that branch
> should be a separate posted mail/patch.  This could also state the gcc error
> that occured, it is not always clear for review (such as the ptrace case).
>
> According to gdb/CONTRIBUTE there should be written ChangeLog entries, that is
> what will be written to gdb/ChangeLog (one writes them as plain text into the
> mail, not directly patching the file gdb/ChangeLog, as the ChangeLog patch
> would get immediately out of scope).  Some requests for comments without
> immediate check-in may got without ChangeLog entry, such as this preview
> patch.
>
> It is not a requirement but the preference is to post the patches inlined in
> the mail text; just I am not sure Thunderbird will not corrupt it, your mail
> body is format=flowed which would corrupt it, OTOH without format=flowed some
> mailers wrap the patch to some fixed column.  So maybe the attachment is the
> least worst for Thunderbird.
>
>
>> --- a/gdb/amd64-linux-nat.c
>> +++ b/gdb/amd64-linux-nat.c
>> @@ -172,7 +172,7 @@ amd64_linux_fetch_inferior_registers (struct target_ops *ops,
>>       {
>>         elf_gregset_t regs;
>>   
>> -      if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
>> +      if (ptrace ((enum __ptrace_request) PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
>>   	perror_with_name (_("Couldn't get registers"));
>>   
>>         amd64_supply_native_gregset (regcache, &regs, -1);
> enum __ptrace_request it is on GNU/Linux but not on other platforms where GDB
> is compilable.  My guess is the right solution could be:
>
> configure.ac:
> -for gdb_arg1 in 'int' 'long'; do
> +for gdb_arg1 in 'enum __ptrace_request' 'int' 'long'; do
>
>
>> --- a/gdb/amd64-tdep.c
>> +++ b/gdb/amd64-tdep.c
>> @@ -762,12 +762,12 @@ amd64_push_arguments (struct regcache *regcache, int nargs,
>>       AMD64_XMM0_REGNUM + 4, AMD64_XMM0_REGNUM + 5,
>>       AMD64_XMM0_REGNUM + 6, AMD64_XMM0_REGNUM + 7,
>>     };
>> -  struct value **stack_args = alloca (nargs * sizeof (struct value *));
>> +  struct value **stack_args = (struct value **) alloca (nargs * sizeof (struct value *));
> Here the line got longer than 80 columns, this is forbidden by GCS:
> 	https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
>
> It is not always clear what is best in such case, it may be in some cases for
> example better to move the initialization from the declaration:
>
>    struct value **stack_args;
>
>    stack_args = (struct value **) alloca (nargs * sizeof (struct value *));
>
>
> Sorry for not reviewing the rest of your patch now, it should be split anyway.
>
>
> Thanks,
> Jan
Thank you for your detailed patch reviews,
I'll address the issues.
Thanks,
Ondrej