From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26511 invoked by alias); 1 Oct 2013 12:57:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 26501 invoked by uid 89); 1 Oct 2013 12:57:01 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2013 12:57:01 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r91CuxJP020500 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 1 Oct 2013 08:56:59 -0400 Received: from [10.34.4.133] (unused-4-133.brq.redhat.com [10.34.4.133]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r91CuvkW032231; Tue, 1 Oct 2013 08:56:57 -0400 Message-ID: <524AC698.9040103@redhat.com> Date: Tue, 01 Oct 2013 12:57:00 -0000 From: Ondrej Oprala User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9 MIME-Version: 1.0 To: Jan Kratochvil CC: "'gdb-patches@sourceware.org'" , sanimir.agovic@intel.com Subject: Re: C++-compat clean build References: <524AB12E.8090209@redhat.com> <20131001125338.GA12847@host2.jankratochvil.net> In-Reply-To: <20131001125338.GA12847@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-10/txt/msg00043.txt.bz2 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) ®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 Thank you for your detailed patch reviews, I'll address the issues. Thanks, Ondrej