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

[Kai Tietz <ktietz70@googlemail.com>]

2011/2/23 Pierre Muller <pierre.muller@ics-cnrs.unistra.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

Applied patch don't have those types any more. I introduced them in
initial patch under assumption structures in include/dwarf2.h should
be host-independent. As now those structures are moved into binutils
private header binutils/dwarf.h there is no need to have here new
types and patch uses bfd_vma/bfd_signed_vma.

So what exactly is the point where you see here still the use of
dw2_vma_t/dw2_svma_t?

Regards,
Kai