This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C++-compat clean build


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]