From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 632 invoked by alias); 1 Oct 2013 13:06:22 -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 618 invoked by uid 89); 1 Oct 2013 13:06:21 -0000 Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2013 13:06:21 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RDNS_NONE,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mga03.intel.com Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by azsmga101.ch.intel.com with ESMTP; 01 Oct 2013 06:06:18 -0700 X-ExtLoop1: 1 Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by fmsmga001.fm.intel.com with ESMTP; 01 Oct 2013 06:06:16 -0700 Received: from irsmsx151.ger.corp.intel.com (163.33.192.59) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.123.3; Tue, 1 Oct 2013 14:04:22 +0100 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.69]) by IRSMSX151.ger.corp.intel.com ([169.254.4.177]) with mapi id 14.03.0123.003; Tue, 1 Oct 2013 14:04:22 +0100 From: "Tedeschi, Walfred" To: Ondrej Oprala , Jan Kratochvil CC: "'gdb-patches@sourceware.org'" , "Agovic, Sanimir" Subject: RE: C++-compat clean build Date: Tue, 01 Oct 2013 13:06:00 -0000 Message-ID: References: <524AB12E.8090209@redhat.com> <20131001125338.GA12847@host2.jankratochvil.net> <524AC698.9040103@redhat.com> In-Reply-To: <524AC698.9040103@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2013-10/txt/msg00044.txt.bz2 Ondrej, I have also seem many GNU style violations in your patch like "=3D" followe= d by two spaces. Parameters not having the right indentation on the next line and so on... Some of them are here, just in case: On line 606 it seems that style is off. Line 734 seems to be too long. On line 791 you removed a line that does not belong to this patch. In many places you have equals followed by two spaces please change for on= ly one, e.g. lines 952 961 970 too many spaces after equal, please verify o= ther places. Same as before: 1007 1008 1017 1035 1200...1210. Line 1104 equal is on the next line. Ax-general.c on the hunk -96,8 -> spaces and tabs are wrong and 460,8 par= ameters indentation.=20 On i386-tdep.c hunk 7799 parameters are off. Too long lines more than 80 chars on the hunk linux-nat.c Remote.c hunks: -587, 8 parameters are off. Best regards, -Fred -----Original Message----- From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware= .org] On Behalf Of Ondrej Oprala Sent: Tuesday, October 01, 2013 2:57 PM To: Jan Kratochvil Cc: 'gdb-patches@sourceware.org'; Agovic, Sanimir Subject: 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=20 >> compile cleanly with -Wc++-compat. >> The idea is to make separate patches for respective subdirs under=20 >> gdb/, unless someone objects ofc. > this is a too huge patch. It should import first archer/tromey/c++=20 > which is already separated into specific parts, that is each commit in=20 > that branch should be a separate posted mail/patch. This could also=20 > 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,=20 > that is what will be written to gdb/ChangeLog (one writes them as=20 > plain text into the mail, not directly patching the file=20 > gdb/ChangeLog, as the ChangeLog patch would get immediately out of=20 > scope). Some requests for comments without immediate check-in may got=20 > without ChangeLog entry, such as this preview patch. > > It is not a requirement but the preference is to post the patches=20 > inlined in the mail text; just I am not sure Thunderbird will not=20 > corrupt it, your mail body is format=3Dflowed which would corrupt it,=20 > OTOH without format=3Dflowed some mailers wrap the patch to some fixed=20 > 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; >>=20=20=20 >> - if (ptrace (PTRACE_GETREGS, tid, 0, (long) ®s) < 0) >> + if (ptrace ((enum __ptrace_request) PTRACE_GETREGS, tid, 0,=20 >> + (long) ®s) < 0) >> perror_with_name (_("Couldn't get registers")); >>=20=20=20 >> amd64_supply_native_gregset (regcache, ®s, -1); > enum __ptrace_request it is on GNU/Linux but not on other platforms=20 > 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, i= nt nargs, >> AMD64_XMM0_REGNUM + 4, AMD64_XMM0_REGNUM + 5, >> AMD64_XMM0_REGNUM + 6, AMD64_XMM0_REGNUM + 7, >> }; >> - struct value **stack_args =3D alloca (nargs * sizeof (struct value=20 >> *)); >> + struct value **stack_args =3D (struct value **) alloca (nargs *=20 >> + 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=20 > cases for example better to move the initialization from the declaration: > > struct value **stack_args; > > stack_args =3D (struct value **) alloca (nargs * sizeof (struct value= =20 > *)); > > > Sorry for not reviewing the rest of your patch now, it should be split an= yway. > > > Thanks, > Jan Thank you for your detailed patch reviews, I'll address the issues. Thanks, Ondrej Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052