Re: [PATCH] readelf: dump the value contained in offset on Rel relocs when -AW are passed

[Nick Clifton]

Hi Andrea,

  Thanks for patch suggestion.  I do have a few comments:

> Here is the suggested patch.  It will print the value at offset on Rel
> relocs; for simplicity, it only does it on 32-bit files, because that's
> what I need, but I can easily extend it; also, 64-bit ELFs tend to use
> Rela relocs where this feature does not make sense.

True.  It felt a little odd only making the change for 32-bit, but less
changes to the code = less chance for new bugs...


> The additional column is displayed when -A (--arch-specific) is passed,
> as suggested, together with -W for wide mode (I can remove this last
> requirement).

I think that that would be good.  (Ie make the change work with and without -W).

Also - I think that it would be neater if you put the extra field at the
end of the line, rather than after the Offset field.  That way the layout
isn't changed so much, and you can add the extra value-calculating-code
to the end of the loop.

> +             value = 0;
> +
> +             if (sec)
> +               {
> +                 file_offset = offset - sec->sh_addr + sec->sh_offset;
> +
> +                 /* Now read value, or set to zero on failure */
> +                 if (!fseek (filedata->handle, file_offset, SEEK_SET))
> +                   if (fread (&value, sizeof (value), 1, filedata->handle) != 
> 1)
> +                     value = 0;

Rather than printing a value of 0 for locations that cannot be reached, it
might be better to print something like ????????.  That way users can tell
at a glance that something is wrong...

When adding a new feature like this, you also need to document it.  Specifically
you need to add a line or two to the binutils/NEWS file, and a full decription
in the readelf section of the binutils/doc/binutils.texi file.

Finally you need to run the binutils testsuite and make sure that the change 
does
not introduce any new failures.  (It should not, since the behaviour is gated on
the -A option, which I think is not widely tested, but you never know).

Cheers
  Nick