This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: C++-compat clean build
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Ondrej Oprala <ooprala at redhat dot com>
- Cc: "'gdb-patches at sourceware dot org'" <gdb-patches at sourceware dot org>
- Date: Tue, 1 Oct 2013 14:53:38 +0200
- Subject: Re: C++-compat clean build
- Authentication-results: sourceware.org; auth=none
- References: <524AB12E dot 8090209 at redhat dot com>
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) ®s) < 0)
> + if (ptrace ((enum __ptrace_request) PTRACE_GETREGS, tid, 0, (long) ®s) < 0)
> perror_with_name (_("Couldn't get registers"));
>
> amd64_supply_native_gregset (regcache, ®s, -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