Objcopy and binary files
Nick Clifton
nickc@redhat.com
Fri Feb 16 10:45:00 GMT 2001
Hi Stefan,
Thanks very much for submitting this patch. I do have a few
comments, but first of all, I have to say that with a patch of this
size, the FSF does need a copyright assignment on file from you
before they can accept it. I am attaching an email form which
you can fill out if you want, in order to apply for the copyright
assignment.
: Two files have to be changed:
:
: a) binary.c in directory bfd
: b) objcopy.c in directory binutils
Actually you also ought to modify two more files in the binutils
directory: binutils.texi (add some documentation of the new command
line switch) and NEWS (add a line describing the new feature of
objcopy).
: Copy this directly in front of the (last) return in this function:
:
: if (bfd_external_binary_architecture != bfd_arch_unknown)
: bfd_set_arch_info(abfd,
: bfd_lookup_arch(bfd_external_binary_architecture,
: 0));
It is wise to be paranoid. In particular I would suggest that you
only call bfd_set_arch_info() if the architecture has not already
been set. ie something like:
if (bfd_get_arch_info (abfd) == NULL
&& bfd_external_binary_architecture != bfd_arch_unknown)
bfd_set_arch_info (abfd,
bfd_lookup_arch (bfd_external_binary_architecture,
NULL));
Also note - when you are writing code for inclusion in GNU software
you should follow the GNU coding standard:
http://www.gnu.org/prep/standards_toc.html
In this particular case I would like to point out that you ought to
leave a space between the end of a function's name and the opening
parenthesis of its argument list.
: Finally, prove (also in this function) behind the case instruction, if
: input target is "binary" and if -B (--binary-architecture) is set. Set
: the global variable from binary.c.
:
: I have placed the test
:
: if (strcmp(input_target, "binary") == 0)
: {
: if (binary_architecture != NULL)
: {
: const bfd_arch_info_type* temp_arch_info =
: bfd_scan_arch(binary_architecture);
:
: if (temp_arch_info != NULL)
: bfd_external_binary_architecture = temp_arch_info->arch;
: else
: non_fatal (_("warning: architecture %s unknown"),
: binary_architecture);
: }
: }
You should also handle the cases where input_target is NULL or some
other string, but binary_architecture is not NULL. Probably the
best thing to issue a warning message saying the the -B switch is
being ignored.
Other than that the patch looks fine.
Cheers
Nick
---------------------------------------------------------------------------
request-assign.future:
Please email the following information to fsf-records@gnu.org, and we
will send you the assignment form for your past and future changes.
Please use your full name as the subject line of the message.
[What is the name of the program or package you're contributing to?]
[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
[Do you have an employer who might have a basis to claim to own
your changes? Do you attend a school which might make such a claim?]
[For the copyright registration, what country are you a citizen of?]
[What year were you born?]
[Please write your email address here.]
[Please write your snail address here.]
[Which files have you changed so far, and which new files have you written
so far?]
---------------------------------------------------------------------------
More information about the Binutils
mailing list