Re: [PATCH] bfd: Add Support for DW_FORM_strx* and DW_FORM_addrx*.

[Jan Beulich <jbeulich@suse.com>]

On 22.04.2022 08:36, Potharla, Rupesh via Binutils wrote:
> Requesting to review the attached patch. The patch provides support for strx* and addrx* forms to the bfd library.

There's a change to decode_line_info() which looks unrelated. If it's
related, you will want to explain this in the description. If it's
not related, you will want to split this into a separate patch with
its own justification. The same looks to repeat in
find_abstract_instance() and yet more places. 

There's a use of bfd_realloc() where you forgot to check for failure.

I consider it at least risky to open-code the conversion from index
to actual value into various callers of read_attribute(), instead of
keeping it in read_attribute_value(), as was clearly originally
intended.

In read_indexed_string(), besides swapping the parameter order for no
apparent reason, you go from a 64-bit type to "unsigned long". Are
you sure this is going to be sufficient on 32-bit architectures? I'd
rather see read_indexed_address() use bfd_uint64_t as well ...

Furthermore read_indexed_address() has a return type of "int", but
you mean to return addresses there. Quite likely this wants to at
least be bfd_vma, if not bfd_uint64_t.

There are also quite a few indentation and padding issues, which you
will want to take care of.

To aid reviewers, please try to supply patches inline rather than as
attachment.

Jan