mkdtemp, mkstemp and bug 2876 fallout

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

Ben pointed out to me today that compiling binutils using current
glibc headers gives warnings in bucomm.c about not using the result of
mkdtemp.  I took a look and discovered some other problems as well..
First, I noticed that make_tempname was missing a call to mktemp.
Secondly, make_tempdir doesn't handle HAVE_DOS_BASED_FILE_SYSTEM as it
is lacking the backslash tests.  Fixed by extracting the code from
make_tempname into a separate function, cleaned up a little.  I
removed the extraordinarily long templates while I was at it.
Thirdly, make_tempname behaves quite differently depending on
HAVE_MKSTEMP.  When mkstemp is used a temp file is created, while
for mktemp no file is created.  To make the situation consistent,
I modified make_tempname to always create the file.
Finally, make_tempdir was only available when HAVE_MKDTEMP.  I think
it's much cleaner if make_tempdir handles directory creation when
!HAVE_MKDTEMP too.  In particular, calling make_tempname for a
directory is bad, and would fail if we have !HAVE_MKDTEMP and
HAVE_MKSTEMP.

	* bucomm.h (make_tempdir): Declare independently of HAVE_MKDTEMP.
	* bucomm.c (template_in_dir): New function, split out from..
	(make_tempname): ..here.  Open the file with O_EXCL if !HAVE_MKSTEMP.
	(make_tempdir): Use template_in_dir.  Handle directory creation
	when !HAVE_MKDTEMP.
	* objcopy.c (MKDIR): Don't define.
	(copy_archive): Use make_tempdir when !HAVE_MKDTEMP too.  Fix
	error message.

Index: binutils/bucomm.h
===================================================================
RCS file: /cvs/src/src/binutils/bucomm.h,v
retrieving revision 1.22
diff -c -p -r1.22 bucomm.h
*** binutils/bucomm.h	13 Oct 2006 09:43:28 -0000	1.22
--- binutils/bucomm.h	10 Jan 2007 11:24:51 -0000
*************** int display_info (void);
*** 200,208 ****
  void print_arelt_descr (FILE *, bfd *, bfd_boolean);
  
  char *make_tempname (char *);
- #if defined(HAVE_MKDTEMP)
  char *make_tempdir (char *);
- #endif
  
  bfd_vma parse_vma (const char *, const char *);
  
--- 200,206 ----
Index: binutils/bucomm.c
===================================================================
RCS file: /cvs/src/src/binutils/bucomm.c,v
retrieving revision 1.26
diff -c -p -r1.26 bucomm.c
*** binutils/bucomm.c	13 Oct 2006 09:43:28 -0000	1.26
--- binutils/bucomm.c	10 Jan 2007 11:24:51 -0000
*************** print_arelt_descr (FILE *file, bfd *abfd
*** 386,496 ****
    fprintf (file, "%s\n", bfd_get_filename (abfd));
  }
  
! /* Return the name of a created temporary file in the same directory as FILENAME.  */
  
! char *
! make_tempname (char *filename)
  {
! #if defined(HAVE_MKSTEMP)
!   static char template[] = "stXXXXXXXXXX";
!   int fd;
! #else
!   static char template[] = "stXXXXXX";
! #endif
    char *tmpname;
!   char *slash = strrchr (filename, '/');
  
  #ifdef HAVE_DOS_BASED_FILE_SYSTEM
    {
      /* We could have foo/bar\\baz, or foo\\bar, or d:bar.  */
!     char *bslash = strrchr (filename, '\\');
  
      if (slash == NULL || (bslash != NULL && bslash > slash))
        slash = bslash;
!     if (slash == NULL && filename[0] != '\0' && filename[1] == ':')
        slash = filename + 1;
    }
  #endif
  
    if (slash != (char *) NULL)
      {
!       char c;
  
-       c = *slash;
-       *slash = 0;
-       tmpname = xmalloc (strlen (filename) + sizeof (template) + 2);
-       strcpy (tmpname, filename);
  #ifdef HAVE_DOS_BASED_FILE_SYSTEM
        /* If tmpname is "X:", appending a slash will make it a root
  	 directory on drive X, which is NOT the same as the current
  	 directory on drive X.  */
!       if (tmpname[1] == ':' && tmpname[2] == '\0')
! 	strcat (tmpname, ".");
  #endif
!       strcat (tmpname, "/");
!       strcat (tmpname, template);
! #if defined(HAVE_MKSTEMP)
!       fd = mkstemp (tmpname);
! #else
!       mktemp (tmpname);
! #endif
!       *slash = c;
      }
    else
      {
        tmpname = xmalloc (sizeof (template));
!       strcpy (tmpname, template);
! #if defined(HAVE_MKSTEMP)
!       fd = mkstemp (tmpname);
! #endif
      }
! #if defined(HAVE_MKSTEMP)
!   if (fd == -1)
      return NULL;
!   close(fd);
  #endif
    return tmpname;
  }
  
! #if defined(HAVE_MKDTEMP)
! /* Return the name of a created temporary directory inside the directory containing FILENAME.  */
  
  char *
  make_tempdir (char *filename)
  {
!   static char template[] = "stXXXXXXXXXX";
!   char *tmpname;
!   char *slash = strrchr (filename, '/');
! 
!   if (slash != (char *) NULL)
!     {
!       char c;
  
!       c = *slash;
!       *slash = 0;
!       tmpname = xmalloc (strlen (filename) + sizeof (template) + 1);
!       strcpy (tmpname, filename);
! #ifdef HAVE_DOS_BASED_FILE_SYSTEM
!       /* If tmpname is "X:", appending a slash will make it a root
!          directory on drive X, which is NOT the same as the current
!          directory on drive X.  */
!       if (tmpname[1] == ':' && tmpname[2] == '\0')
!         strcat (tmpname, ".");
  #endif
-       strcat (tmpname, "/");
-       strcat (tmpname, template);
-       mkdtemp (tmpname);
-       *slash = c;
-    }
-   else
-     {
-       tmpname = xmalloc (sizeof (template));
-       strcpy (tmpname, template);
-       mkdtemp (tmpname);
-     }
    return tmpname;
  }
- #endif /* HAVE_MKDTEMP */
  
  /* Parse a string into a VMA, with a fatal error if it can't be
     parsed.  */
--- 386,487 ----
    fprintf (file, "%s\n", bfd_get_filename (abfd));
  }
  
! /* Return a path for a new temporary file in the same directory
!    as file PATH.  */
  
! static char *
! template_in_dir (const char *path)
  {
! #define template "stXXXXXX"
!   char *slash = strrchr (path, '/');
    char *tmpname;
!   size_t len;
  
  #ifdef HAVE_DOS_BASED_FILE_SYSTEM
    {
      /* We could have foo/bar\\baz, or foo\\bar, or d:bar.  */
!     char *bslash = strrchr (path, '\\');
  
      if (slash == NULL || (bslash != NULL && bslash > slash))
        slash = bslash;
!     if (slash == NULL && path[0] != '\0' && path[1] == ':')
        slash = filename + 1;
    }
  #endif
  
    if (slash != (char *) NULL)
      {
!       len = slash - path;
!       tmpname = xmalloc (len + sizeof (template) + 2);
!       memcpy (tmpname, path, len);
  
  #ifdef HAVE_DOS_BASED_FILE_SYSTEM
        /* If tmpname is "X:", appending a slash will make it a root
  	 directory on drive X, which is NOT the same as the current
  	 directory on drive X.  */
!       if (len == 2 && tmpname[1] == ':')
! 	tmpname[len++] = '.';
  #endif
!       tmpname[len++] = '/';
      }
    else
      {
        tmpname = xmalloc (sizeof (template));
!       len = 0;
      }
! 
!   memcpy (tmpname + len, template, sizeof (template));
!   return tmpname;
! #undef template
! }
! 
! /* Return the name of a created temporary file in the same directory
!    as FILENAME.  */
! 
! char *
! make_tempname (char *filename)
! {
!   char *tmpname = template_in_dir (filename);
!   int fd;
! 
! #ifdef HAVE_MKSTEMP
!   fd = mkstemp (tmpname);
! #else
!   tmpname = mktemp (tmpname);
!   if (tmpname == NULL)
      return NULL;
!   fd = open (tmpname, O_RDWR | O_CREAT | O_EXCL, 0600);
  #endif
+   if (fd == -1)
+     return NULL;
+   close (fd);
    return tmpname;
  }
  
! /* Return the name of a created temporary directory inside the
!    directory containing FILENAME.  */
  
  char *
  make_tempdir (char *filename)
  {
!   char *tmpname = template_in_dir (filename);
  
! #ifdef HAVE_MKDTEMP
!   return mkdtemp (tmpname);
! #else
!   tmpname = mktemp (tmpname);
!   if (tmpname == NULL)
!     return NULL;
! #if defined (_WIN32) && !defined (__CYGWIN32__)
!   if (mkdir (tmpname) != 0)
!     return NULL;
! #else
!   if (mkdir (tmpname, 0700) != 0)
!     return NULL;
  #endif
    return tmpname;
+ #endif
  }
  
  /* Parse a string into a VMA, with a fatal error if it can't be
     parsed.  */
Index: binutils/objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.103
diff -c -p -r1.103 objcopy.c
*** binutils/objcopy.c	13 Oct 2006 09:43:29 -0000	1.103
--- binutils/objcopy.c	10 Jan 2007 11:24:57 -0000
*************** copy_object (bfd *ibfd, bfd *obfd)
*** 1764,1778 ****
    return TRUE;
  }
  
- #if ! defined(HAVE_MKDTEMP)
- #undef MKDIR
- #if defined (_WIN32) && !defined (__CYGWIN32__)
- #define MKDIR(DIR, MODE) mkdir (DIR)
- #else
- #define MKDIR(DIR, MODE) mkdir (DIR, MODE)
- #endif
- #endif
- 
  /* Read each archive element in turn from IBFD, copy the
     contents to temp file, and keep the temp file handle.
     If 'force_output_target' is TRUE then make sure that
--- 1764,1769 ----
*************** copy_archive (bfd *ibfd, bfd *obfd, cons
*** 1794,1812 ****
    char * dir;
  
    /* Make a temp directory to hold the contents.  */
- #if defined(HAVE_MKDTEMP)
    dir = make_tempdir (bfd_get_filename (obfd));
- 
    if (dir == NULL)
        fatal (_("cannot create tempdir for archive copying (error: %s)"),
  	   strerror (errno));
- #else
-   dir = make_tempname (bfd_get_filename (obfd));
- 
-   if (MKDIR (dir, 0700) != 0)
-     fatal (_("cannot mkdir %s for archive copying (error: %s)"),
- 	   dir, strerror (errno));
- #endif
  
    obfd->has_armap = ibfd->has_armap;
  
--- 1785,1794 ----
*************** copy_archive (bfd *ibfd, bfd *obfd, cons
*** 1833,1849 ****
        /* If the file already exists, make another temp dir.  */
        if (stat (output_name, &buf) >= 0)
  	{
- #if defined(HAVE_MKDTEMP)
  	  output_name = make_tempdir (output_name);
  	  if (output_name == NULL)
! 	    fatal (_("cannot create temporary dir '%s' for archive copying (error: %s)"),
! 		   output_name, strerror (errno));
! #else
! 	  output_name = make_tempname (output_name);
! 	  if (MKDIR (output_name, 0700) != 0)
! 	    fatal (_("cannot mkdir %s for archive copying (error: %s)"),
! 		   output_name, strerror (errno));
! #endif
  
  	  l = xmalloc (sizeof (struct name_list));
  	  l->name = output_name;
--- 1815,1824 ----
        /* If the file already exists, make another temp dir.  */
        if (stat (output_name, &buf) >= 0)
  	{
  	  output_name = make_tempdir (output_name);
  	  if (output_name == NULL)
! 	    fatal (_("cannot create tempdir for archive copying (error: %s)"),
! 		   strerror (errno));
  
  	  l = xmalloc (sizeof (struct name_list));
  	  l->name = output_name;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre