More tidies to objcopy archive handling

This makes sure copy_archive exits with ibfd and obfd closed.  Error
paths didn't do that, leading to memory leaks.  None of this matters
very much.

	* objcopy.c (copy_archive): bfd_close ibfd and obfd on error
	return paths.  Remove braces around "list" free.
	(copy_file): Don't close invalid file descriptor.

1 files changed, 32 insertions, 28 deletions

diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index c931f67..beb655c 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -3615,9 +3615,11 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
     } *list, *l;
   bfd **ptr = &obfd->archive_head;
   bfd *this_element;
-  char *dir;
+  char *dir = NULL;
   char *filename;
 
+  list = NULL;
+
   /* PR 24281: It is not clear what should happen when copying a thin archive.
      One part is straight forward - if the output archive is in a different
      directory from the input archive then any relative paths in the library
@@ -3636,7 +3638,7 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
       bfd_set_error (bfd_error_invalid_operation);
       bfd_nonfatal_message (NULL, ibfd, NULL,
 			    _("sorry: copying thin archives is not currently supported"));
-      return;
+      goto cleanup_and_exit;
     }
 
   /* Make a temp directory to hold the contents.  */
@@ -3654,8 +3656,6 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
   if (deterministic)
     obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
 
-  list = NULL;
-
   this_element = bfd_openr_next_archived_file (ibfd, NULL);
 
   if (!bfd_set_format (obfd, bfd_get_format (ibfd)))
@@ -3797,44 +3797,46 @@ copy_archive (bfd *ibfd, bfd *obfd, const char *output_target,
     }
   *ptr = NULL;
 
+ cleanup_and_exit:
   filename = xstrdup (bfd_get_filename (obfd));
   if (!(status == 0 ? bfd_close : bfd_close_all_done) (obfd))
     {
+      if (!status)
+	bfd_nonfatal_message (filename, NULL, NULL, NULL);
       status = 1;
-      bfd_nonfatal_message (filename, NULL, NULL, NULL);
     }
   free (filename);
 
   filename = xstrdup (bfd_get_filename (ibfd));
   if (!bfd_close (ibfd))
     {
+      if (!status)
+	bfd_nonfatal_message (filename, NULL, NULL, NULL);
       status = 1;
-      bfd_nonfatal_message (filename, NULL, NULL, NULL);
     }
   free (filename);
 
- cleanup_and_exit:
   /* Delete all the files that we opened.  */
-  {
-    struct name_list * next;
-
-    for (l = list; l != NULL; l = next)
-      {
-	if (l->obfd == NULL)
-	  rmdir (l->name);
-	else
-	  {
-	    bfd_close (l->obfd);
-	    unlink (l->name);
-	  }
-	free ((char *) l->name);
-	next = l->next;
-	free (l);
-      }
-  }
+  struct name_list *next;
+  for (l = list; l != NULL; l = next)
+    {
+      if (l->obfd == NULL)
+	rmdir (l->name);
+      else
+	{
+	  bfd_close (l->obfd);
+	  unlink (l->name);
+	}
+      free ((char *) l->name);
+      next = l->next;
+      free (l);
+    }
 
-  rmdir (dir);
-  free (dir);
+  if (dir)
+    {
+      rmdir (dir);
+      free (dir);
+    }
 }
 
 /* The top-level control.  */
@@ -3933,7 +3935,8 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
 
       if (obfd == NULL)
 	{
-	  close (ofd);
+	  if (ofd >= 0)
+	    close (ofd);
 	  bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
 	  bfd_close (ibfd);
 	  status = 1;
@@ -3966,7 +3969,8 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
 
       if (obfd == NULL)
  	{
-	  close (ofd);
+	  if (ofd >= 0)
+	    close (ofd);
  	  bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
 	  bfd_close (ibfd);
  	  status = 1;