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

NightStrike
2011-02-21T12:37:55Z

Ping

On Fri, Feb 18, 2011 at 4:50 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/2/17 Jakub Jelinek <jakub@redhat.com>:
>> On Thu, Feb 17, 2011 at 08:06:53PM +0100, Kai Tietz wrote:
>>> > I'd prefer if we could move those binutils specific internal implementation
>>> > details out of the dwarf2.h header, keep it really just for the stuff
>>> > defined in the standard, and add a new binutils specific header somewhere
>>> > else which would contain binutils specific details.
>>>
>>> Well, so we make dwarf2.h in include just host-specific and isn't able
>>> to cover things for foreign target, like 32-bit host can't use this
>>> header for 64-bit dwarf2 information. I am fine by this, but
>>> nevertheless is then the use of 'unsigned long' type within this
>>> header still an issue. Here should be used instead then 'intptr_t'. As
>>> on LLP64 unsigned long isn't wide-enough.
>>
>> I mean that none of the DWARF2_Internal types should be in the
>> include/dwarf2.h headers (that's implementation specific stuff),
>> and the DWARF2_External types are questionable too, to me they look
>> very much like implementation detail too.  E.g. DWARF2_External_LineInfo
>> type is dubious, because it might somehow describe version 2 or 3
>> .debug_line header, but version 4 looks differently, etc.
>> So IMHO the DWARF2_External stuff should be moved too.
>> And the guard should be probably renamed from _ELF_DWARF2_H to _DWARF2_H,
>> I don't see anything ELF specific in the header.
>>
>>        Jakub
>>
>
> So I reworked my patch as Jakub suggested and retested it for
> x86_64-w64-mingw32, i686-w64-mingw32, and x86_64-pc-linux-gnu for gcc,
> binutils, and gdb. Ok for apply?
>
> binutils, gdb, gcc
>
> ChangeLog include/
>
> 2011-02-18  Kai Tietz
>
>                 * dwarf2.h (_ELF_DWARF2_H): Renamed to
>                 _DWARF2_H.
>                 (DWARF2_External_LineInfo, DWARF2_Internal_LineInfo,
>                 DWARF2_External_PubNames, DWARF2_Internal_PubNames,
>                 DWARF2_External_CompUnit, DWARF2_Internal_CompUnit,
>                 DWARF2_External_ARange, DWARF2_Internal_ARange): Removed.
>
>
> ChangeLog binutils/
>
> 2011-02-18  Kai Tietz
>
>                 * dwarf.c (read_leb128): Use dw2_vma_t instead of
>                 long type.
>                 (process_extended_line_op): Use for adr bfd_vma
>                 type and print those typed values via BFD_VMA_FMT.
>                 (fetch_indirect_string): Adjust offset's type.
>                 (decode_location_expression): Adjust argument types
>                 and uvalue type.
>                 (read_and_display_attr_value): Likewise.
>                 (read_and_display_attr): Likewise.
>                 (decode_location_expression): Adjust printf format.
>                 (process_debug_info): Likewise.
>                 (display_debug_lines_raw): Likewise.
>                 (display_debug_lines_decoded): Likewise.
>                 (display_debug_pubnames): Likewise.
>                 (display_debug_loc): Likewise.
>                 (display_debug_aranges): Likewise.
>                 * dwarf.h (DWARF2_External_LineInfo,
>                 DWARF2_Internal_LineInfo, DWARF2_External_PubNames,
>                 DWARF2_Internal_PubNames, DWARF2_External_CompUnit,
>                 DWARF2_Internal_CompUnit, DWARF2_External_ARange,
>                 DWARF2_Internal_ARange): Added..
>                 (read_leb128): Adjust return type.
>
> Regards,
> Kai
>
> PS: A lot of those moved structures aren't used at all in binutils
> dwarf.c (which is the only consumer of some of those structures as a
> grep over binutils/gdb/gcc had shown). So maybe the unused could be
> removed in a second patch completely.
>