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