Adding options for redefining symbols
Nick Clifton
nickc@redhat.com
Wed Jun 26 14:01:00 GMT 2019
Hi Ivan,
First of all thank you for your interest in the binutils, and
posting this patch.
> I found myself in need of redefining only the symbols that correspond
> to defined functions, not the ones that are undefined declarations.
I have to say my immediate thought was "why bother ?". There are
already --redefine-sym and --redefine-syms options, so why not use
those. I can see you might want to have a check that the symbol
really is defined (or undefined) before changing it. But you could
achieve that with a small script inserted before calling objcopy,
and then there would be no need to patch anything.
> - Is this kind of feature a feasable candidate?
Well yes and no. Yes, it is feasible because you have already
written a patch that does it. But no, because as mentioned above
it might be possible to achieve the same goal without needing to
patch anything.
> - Setting redefine_mode uses switch fallthroughs, is this a
> readability no-no?
Nope that is fine.
> - Do I need to find similar alternatives for "--redefine-syms=<filename>"?
For consistency's sake I would say yes. There is a limit on
the length of a command line, and if the feature needs to be
used for lots of symbols, then having this alternative would
be really useful.
> Example usage, first redefining for using implementation_a, then
> changing the same object file to instead use implementation_b:
>
> $ objcopy --redefine-sym implementation_a=do_work
> $ objcopy --redefine-defined-sym do_work=implementation_a
> --redefine-sym implementation_b=do_work
Now this confuses me. The first command line makes sense, but
the second one appears to say "rename function 'do_work' to be
called 'implementation_a' and rename function 'implementation_b'
to be called 'do_work'". But this does not match your description
of using implementation_b.
I also have a comment on the patch itself:
+ char mode;
I would suggest that you use an enum here, rather than a char
and some magic numeric values. It does make the patch slightly
larger, but also easier to read.
One final point - are you aware that in order to contribute a
patch like this you will need an FSF copyright assignment in
place ?
Cheers
Nick
More information about the Binutils
mailing list