This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]