Should strip gnore the unknown files in archive?

Nick Clifton nickc@redhat.com
Mon Jun 13 12:36:00 GMT 2005


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



More information about the Binutils mailing list