Re: [patch] better error output from ar when given a directory

[Alan Modra <amodra at bigpond dot net dot au>]

On Sat, Oct 21, 2006 at 08:10:40PM -0400, Mike Frysinger wrote:
> if you feed a directory to ar when trying to create an archive, the error is a 
> bit misleading:
> $ mkdir foo
> $ touch foo.c
> $ ar rcv foo.a foo.c foo
> a - foo.c
> a - foo
> ar: foo.a: Is a directory
[snip]

> --- bfd/opncls.c
> +++ bfd/opncls.c
> @@ -182,6 +182,15 @@ bfd_fopen (const char *filename, const c
>  {
>    bfd *nbfd;
>    const bfd_target *target_vec;
> +  struct stat s;
> +
> +  if ((fd != -1 && fstat (fd, &s) == 0) ||
> +      (fd == -1 && stat (filename, &s) == 0))
> +    if (S_ISDIR(s.st_mode))
> +      {
> +        bfd_set_error (bfd_error_file_not_recognized);
> +        return NULL;
> +      }
>  
>    nbfd = _bfd_new_bfd ();
>    if (nbfd == NULL)

I don't think this is a very good solution, as it only catches one
sort of read error.  The real problem is that errors from bfd_close
are assumed to be from writing the output.  However, files to be added
to archives aren't read until _bfd_write_archive_contents is called
from bfd_close.  We need some way to report possible read errors on
the input files.

How about the following?  I won't commit this immediately to give
time for feedback.

	* bfd.c (bfd_error_type): Add bfd_error_on_input.
	(input_bfd, input_error): New static vars.
	(bfd_set_error): Handle bfd_error_on_input.
	(bfd_errmsg): Likewise.
	(bfd_perror): Simplify.
	* archive.c (_bfd_write_archive_contents): Report errors on input.

Index: bfd/bfd.c
===================================================================
RCS file: /cvs/src/src/bfd/bfd.c,v
retrieving revision 1.88
diff -u -p -r1.88 bfd.c
--- bfd/bfd.c	16 Sep 2006 18:12:13 -0000	1.88
+++ bfd/bfd.c	27 Oct 2006 14:18:08 -0000
@@ -271,6 +271,7 @@ CODE_FRAGMENT
 .  bfd_error_bad_value,
 .  bfd_error_file_truncated,
 .  bfd_error_file_too_big,
+.  bfd_error_on_input,
 .  bfd_error_invalid_error_code
 .}
 .bfd_error_type;
@@ -278,6 +279,8 @@ CODE_FRAGMENT
 */
 
 static bfd_error_type bfd_error = bfd_error_no_error;
+static bfd *input_bfd = NULL;
+static bfd_error_type input_error = bfd_error_no_error;
 
 const char *const bfd_errmsgs[] =
 {
@@ -300,6 +303,7 @@ const char *const bfd_errmsgs[] =
   N_("Bad value"),
   N_("File truncated"),
   N_("File too big"),
+  N_("Error reading "),
   N_("#<Invalid error code>")
 };
 
@@ -325,16 +329,30 @@ FUNCTION
 	bfd_set_error
 
 SYNOPSIS
-	void bfd_set_error (bfd_error_type error_tag);
+	void bfd_set_error (bfd_error_type error_tag, ...);
 
 DESCRIPTION
 	Set the BFD error condition to be @var{error_tag}.
+	If @var{error_tag} is bfd_error_on_input, then this function
+	takes two more parameters, the input bfd where the error
+	occurred, and the bfd_error_type error.
 */
 
 void
-bfd_set_error (bfd_error_type error_tag)
+bfd_set_error (bfd_error_type error_tag, ...)
 {
   bfd_error = error_tag;
+  if (error_tag == bfd_error_on_input)
+    {
+      /* This is an error that occurred during bfd_close when
+	 writing an archive, but on one of the input files.  */
+      va_list ap;
+
+      va_start (ap, error_tag);
+      input_bfd = va_arg (ap, bfd *);
+      input_error = va_arg (ap, int);
+      va_end (ap);
+    }
 }
 
 /*
@@ -355,6 +373,10 @@ bfd_errmsg (bfd_error_type error_tag)
 #ifndef errno
   extern int errno;
 #endif
+  if (error_tag == bfd_error_on_input)
+    return concat (_(bfd_errmsgs [error_tag]), input_bfd->filename, ": ",
+		   bfd_errmsg (input_error), NULL);
+
   if (error_tag == bfd_error_system_call)
     return xstrerror (errno);
 
@@ -382,16 +404,10 @@ DESCRIPTION
 void
 bfd_perror (const char *message)
 {
-  if (bfd_get_error () == bfd_error_system_call)
-    /* Must be a system error then.  */
-    perror ((char *) message);
+  if (message == NULL || *message == '\0')
+    fprintf (stderr, "%s\n", bfd_errmsg (bfd_get_error ()));
   else
-    {
-      if (message == NULL || *message == '\0')
-	fprintf (stderr, "%s\n", bfd_errmsg (bfd_get_error ()));
-      else
-	fprintf (stderr, "%s: %s\n", message, bfd_errmsg (bfd_get_error ()));
-    }
+    fprintf (stderr, "%s: %s\n", message, bfd_errmsg (bfd_get_error ()));
 }
 
 /*
Index: bfd/archive.c
===================================================================
RCS file: /cvs/src/src/bfd/archive.c,v
retrieving revision 1.46
diff -u -p -r1.46 archive.c
--- bfd/archive.c	16 Sep 2006 18:12:13 -0000	1.46
+++ bfd/archive.c	27 Oct 2006 14:30:00 -0000
@@ -1647,14 +1647,14 @@ _bfd_write_archive_contents (bfd *arch)
       if (bfd_write_p (current))
 	{
 	  bfd_set_error (bfd_error_invalid_operation);
-	  return FALSE;
+	  goto input_err;
 	}
       if (!current->arelt_data)
 	{
 	  current->arelt_data =
 	    bfd_ar_hdr_from_filesystem (arch, current->filename, current);
 	  if (!current->arelt_data)
-	    return FALSE;
+	    goto input_err;
 
 	  /* Put in the file name.  */
 	  BFD_SEND (arch, _bfd_truncate_arname,
@@ -1716,7 +1716,7 @@ _bfd_write_archive_contents (bfd *arch)
 	  != sizeof (*hdr))
 	return FALSE;
       if (bfd_seek (current, (file_ptr) 0, SEEK_SET) != 0)
-	return FALSE;
+	goto input_err;
       while (remaining)
 	{
 	  unsigned int amt = DEFAULT_BUFFERSIZE;
@@ -1726,8 +1726,8 @@ _bfd_write_archive_contents (bfd *arch)
 	  if (bfd_bread (buffer, amt, current) != amt)
 	    {
 	      if (bfd_get_error () != bfd_error_system_call)
-		bfd_set_error (bfd_error_malformed_archive);
-	      return FALSE;
+		bfd_set_error (bfd_error_file_truncated);
+	      goto input_err;
 	    }
 	  if (bfd_bwrite (buffer, amt, arch) != amt)
 	    return FALSE;
@@ -1760,6 +1760,10 @@ _bfd_write_archive_contents (bfd *arch)
     }
 
   return TRUE;
+
+ input_err:
+  bfd_set_error (bfd_error_on_input, current, bfd_get_error ());
+  return FALSE;
 }
 
 /* Note that the namidx for the first symbol is 0.  */

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre