RE: [RFC patch]: Adjust the use of 'long' type in dwarf2.h header

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

  Kai,

please excuse me for asking this so late,
but I don't really understand why you had to introduce
a new 'dwarf_vma' type when we already have a 'bfd_signed_vma'
type that must be signed 64-bit as soon as 
BFD64 macro is defined...
See definitions in bfd/bfd-in.h or bfd/bfd-in2.h

This macro should be set if any of the supported target is a 64-bit target
so it should be possible to use that type, no?

  But of course, I might be missing some important point,
could you please explain to me in which case
a 64-bit dwarf_vma could be necessary without BFD64 being set.


Pierre Muller
GDB pascal language maintainer



> -----Message d'origine-----
> De?: binutils-owner@sourceware.org [mailto:binutils-
> owner@sourceware.org] De la part de Kai Tietz
> Envoyé?: mercredi 23 février 2011 10:00
> À?: Nick Clifton
> Cc?: Pierre Muller; GCC Patches; Binutils; gdb; Jakub Jelinek; Joel
> Brobecker
> Objet?: Re: [RFC patch]: Adjust the use of 'long' type in dwarf2.h
> header
> 
> 2011/2/22 Nick Clifton <nickc@redhat.com>:
> > Hi Kai,
> >
> >
> >> So this version uses new function dwarf_vma_print function for
> >> printf-messages, which are getting localized.
> >>
> >> Tested for x86_64-w64-mingw32 and x86_64-pc-linux-gnu. Ok for apply?
> >
> >> +static const char *
> >> +dwarf_vma_print (const char *fmtch, bfd_vma value)
> >
> > I think that I prefer your suggestion of dwarf_vmatoa.
> >
> >> +{
> >> + ?static int buf_pos = 0;
> >> + ?static struct dwarf_vma_print_buf {
> >> + ? ?char place[64];
> >> + ?} buf[16];
> >
> > Ideally you should have a comment here explaining why you have an
> array of
> > these string buffers.
> >
> >> + ?char fmt[32];
> >> + ?char *ret;
> >> +
> >> + ?sprintf (fmt, "%%%s%s", BFD_VMA_FMT, fmtch);
> >> + ?ret = &buf[buf_pos++].place[0];
> >
> > Or, more simply:
> >
> > ?ret = buf[buf_pos++].place;
> >
> >> + ?buf_pos &= 15;
> >
> > That "15" should be: "ARRAY_SIZE (buf) - 1", and it would be safer to
> write:
> >
> > ?buf_pos %= ARRAY_SIZE (buf);
> >
> > and leave it to the compiler to optimize this into an AND operation
> if it
> > can.
> >
> >> snprintf (ret, 64, fmt, value);
> >
> > And the "64" here should be "sizeof (buf[0].place)".
> >
> > Cheers
> > ?Nick
> >
> >
> 
> Hello Nick,
> 
> adjust the binutils part as you suggested. Applied to binutils and gcc
> (rev. 170433) tree.
> 
> Regards,
> Kai