More tidies to objcopy archive handling
authorAlan Modra <amodra@gmail.com>
Fri, 14 Jul 2023 01:17:30 +0000 (10:47 +0930)
committerAlan Modra <amodra@gmail.com>
Fri, 14 Jul 2023 02:23:48 +0000 (11:53 +0930)
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.

binutils/objcopy.c

index c931f67a224d766e60180348dbb178145af954fd..beb655cafc226f7a00f0aa4f895db57c420a76a9 100644 (file)
@@ -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;