This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Should strip gnore the unknown files in archive?


Hi H. J.

I don't mind error on "strip foo.c". But I don't think error on
foo.c in an archive is nice. A warning is better.

Agreed. I assume that you have a patch that does this ?

Here is the patch.

Could you make a couple of changes please:


+const char *
+bfd_archive_filename (bfd *abfd)

Please could you call this function bfd_get_archive_filename() for consistency with bfd_get_filename().


Also - I think that there ought to be a comment here and in the bucomm.h header pointing out that this function returns the name in a static buffer (and hence it should not be called twice in the same expression).

+      return buf;
+    }
+  else
+    return bfd_get_filename (abfd);

In terms of programming clarity the "else" is not needed. In fact if you reversed the sense of the if() statement you would eliminate a level of indentation as well. [This is just a suggestion, not a requirement].



+#define BUFSIZE 8192

This constant is also defined in ar.c. Perhaps it ought to be defined once, in a header somewhere ? [Just a suggestion].


	  || bfd_get_arch (ibfd) != bfd_get_arch (obfd)))
     {
       if (bfd_get_arch (ibfd) == bfd_arch_unknown)
-	fatal (_("Unable to recognise the format of the input file %s"),
-	       bfd_get_filename (ibfd));
+	non_fatal (_("Unable to recognise the format of the input file `%s'"),
+		   bfd_archive_filename (ibfd));
       else

Perhaps I have missed something here, but it looks to me like your patch will always return a warning for an input file which is in an unrecognised format, even if this input file is not part of an archive. We still want an error if the input file is unrecognised and on its own.


+    printf (_("copy from `%s'(%s) to `%s'(%s)\n"),
+	    bfd_archive_filename (ibfd), bfd_get_target (ibfd),
 	    bfd_get_filename (obfd), bfd_get_target (obfd));

The syntax of the printed message looks slightly confusing to me. If IBFD is an archive then bfd_archive_filename() will return a string containing parentheses which make the output look like `FOO(BAR)'(TYPE). I think it would be better if we used square brackets to enclose the target, as in:


copy from `%s'[%s] to `%s'[%s]

Cheers
  Nick


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]