Should strip gnore the unknown files in archive?

[H. J. Lu]

On Mon, Jun 13, 2005 at 01:38:59PM +0100, Nick Clifton wrote:
> 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().

Sure.

> 
> 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).

OK.

> 
> >+      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
> 

I will see.

> 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.

copy_object won't be called on non-archive if it is unknown.

> 
> >+    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]

I copied it from copy_object. Should it also change copy_object to
make it consistent?


H.J.