From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5325 invoked by alias); 25 Feb 2011 12:23:53 -0000 Received: (qmail 5317 invoked by uid 22791); 25 Feb 2011 12:23:52 -0000 X-SWARE-Spam-Status: No, hits=0.3 required=5.0 tests=AWL,BAYES_50,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-qw0-f41.google.com (HELO mail-qw0-f41.google.com) (209.85.216.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Feb 2011 12:23:47 +0000 Received: by qwd7 with SMTP id 7so1563149qwd.0 for ; Fri, 25 Feb 2011 04:23:46 -0800 (PST) MIME-Version: 1.0 Received: by 10.229.184.204 with SMTP id cl12mr1838924qcb.31.1298636625896; Fri, 25 Feb 2011 04:23:45 -0800 (PST) Received: by 10.229.89.197 with HTTP; Fri, 25 Feb 2011 04:23:45 -0800 (PST) In-Reply-To: <5095785081977025060@unknownmsgid> References: <-8460070221060995487@unknownmsgid> <-6930711422310680743@unknownmsgid> <4D63D49D.90101@redhat.com> <-2339605939192327273@unknownmsgid> <-3886800211494155692@unknownmsgid> <1561346207520594884@unknownmsgid> <5095785081977025060@unknownmsgid> Date: Fri, 25 Feb 2011 12:23:00 -0000 Message-ID: Subject: Re: [RFC] Use only dwarf_vma types in dwarf code (was RE: [RFC patch]: Adjust the use of 'long' type in dwarf2.h header) From: Kai Tietz To: Pierre Muller Cc: "H.J. Lu" , Binutils Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-02/txt/msg00307.txt.bz2 2011/2/25 Pierre Muller : > =A0This patch RFC is follow up of the > thread concerning problems with the act that > readelf defines BFD64 under other conditions > than dwarf.c leading to potential problems. > > =A0The idea is to completely remove bfd_vma and dwarf_signed_vma > from dwarf.h and dwarf.c sources. > > =A0The patch is pretty mechanical, but there are several > points that probably needs discussion: > > =A01) This basically makes use of 8-byte for dwarf_vma and dwarf_signed_v= ma > for almost all modern machines/compilers. > =A0It could still impact performance for system where int64 > computation is slow, I have no idea if this can really be a problem or no= t. > > 2) I also tried to remove (long) and (unsigned long) types > whenever they seemed to me to be used for something that could > be a target address or offset and replaced it by a dwarf_{signed_}vma typ= e. > (This is also related to the Mingw64 target issue for > which 'long' is 4-byte whereas 'void *' is 8-byte). > But I might not have caught all occurrences and might have > misunderstood some other cases. > =A0An example is the address field of the Machine_State_Registers, > I don't know if this relates really to a target address... > > =A0Another important point is that I have no machine > that can run the testsuite on my patch, so that it might very well > introduce regression that I am unaware of. > > =A0Anyhow, I will be pleased to get comments > on this patch. > > > Pierre Muller > GDB pascal language maintainer > Index: binutils/dwarf.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/binutils/dwarf.c,v > retrieving revision 1.84 > diff -u -p -r1.84 dwarf.c > --- binutils/dwarf.c =A0 =A023 Feb 2011 08:52:33 -0000 =A0 =A0 =A01.84 > +++ binutils/dwarf.c =A0 =A025 Feb 2011 10:12:30 -0000 > @@ -106,6 +106,7 @@ static void > =A0print_dwarf_vma (dwarf_vma val, unsigned byte_size) > =A0{ > =A0 static char buff[18]; > + =A0int offset; > > =A0 /* Printf does not have a way of specifiying a maximum field width fo= r an > =A0 =A0 =A0integer value, so we print the full value into a buffer and th= en > select > @@ -120,11 +121,29 @@ print_dwarf_vma (dwarf_vma val, unsigned > =A0 snprintf (buff, sizeof (buff), "%16.16lx ", val); > =A0#endif > > - =A0fputs (buff + (byte_size =3D=3D 4 ? 8 : 0), stdout); > + =A0if (byte_size =3D=3D 0) > + =A0 =A0offset =3D 0; > + =A0else > + =A0 =A0if (byte_size > 0 && byte_size <=3D8) Whitespace issue. should be ... && byte_size <=3D 8) > + =A0 =A0 =A0offset =3D 16 - 2 * byte_size; > + =A0 =A0else > + =A0 =A0 =A0error("Wrong size in print_dwarf_vma"); > + > + =A0fputs (buff + offset, stdout); > =A0} > > +#if __STDC_VERSION__ >=3D 199901L || (defined(__GNUC__) && __GNUC__ >=3D= 2) > +#ifndef __MSVCRT__ Please don't use here __MSVCRT__, instead use #if !defined (__MSVCRT__) && !defined (__MINGW32__) As __MSVCRT__ is just declared by VC, and __MINGW32__ for gcc. IMHO __MINGW32__ should be the only guard necessary here, as VC neither defines __STDC_VERSION__ > 199901L and it doesn't defines __GNUC__, too. > +#define =A0DWARF_VMA_FMT "ll" > +#else > +#define =A0DWARF_VMA_FMT "I64" > +#endif > +#else > +#define =A0DWARF_VMA_FMT "l" > +#endif > + > -bfd_vma > +static char * > +dwarf_svmatoa (const char *fmtch, dwarf_signed_vma value) Why you are using here dwarf_svmatoa? It should be dwarf_vmatoa As the formatter is specified here as argument and in dwarf_vmatoa the formatter-buffer is constructed, I see no point in adding a signed variant. This is just a first glance, and well, I can't approve this patch. But in general I appreachiate your modification. Regards, Kai