[discussion/patch] use mkstemps/mkdtemps instead of mktemp.

[Muthukumar Ratty <muthu at iqmail dot net>]

Hi All,
First things first, I am a newbie to binutils and this is my first ever
patch. So I would appreciate if as many expert eyes can review this one.
This is not a final patch but more like a discussion to see if what I have
done is ok or need changes etc. Also I am new to GNU coding style so let me
know if something is not ok there.

What I have done is
1. Added mkdtemps to libiberty. Its same as mkstemps except it uses MKDIR
to create directory.

2. bucomm.c
	A function, get_basetemplate which is same as make_tempname except
it wont call mktemp but returns template.

	Added two functions

	+/* Creates a temporary file in the same directory as NAME, and returns
	+   its file descriptor.  NEWNAME is set to the path of the file.
	+   On failure, -1 is returned.  */
	+
	+int
	+create_tempfile (name, newname)

	this one uses mkstemps

	and
	+/* Creates a temporary directory in the same directory as NAME.
	+   NEWNAME is set to the path of the diretory. On success 0 (zero)
	+   is returned and on failure, -1 is returned.  */
	+
	+int
	+create_tempdir (name, newname)

	this one uses mkdtemps.

3. ar.c
	Instead of make_tempname it now uses create_tempfile.
	I wanted to use the returned fd and call bfd_fdopenr with that but
        it fails when it goes to bfd_close:BFD_SEND_FMT for the last time.
        So right now I am closing the tempfd and use the bfd_openw as
        before. Any help with that is really appreciated.

4. objcopy.c
	Instead of make_tempname for directories, it now uses
        create_tempdir
	Instead of make_tempname for files it now uses create_tempfile and
        closes the fd returned.

------------------------------PATCH-------------------------------------



--- ../../src-cvs/binutils/ar.c	Thu Jul  3 00:57:25 2003
+++ ar.c	Thu Jul  3 11:45:23 2003
@@ -1083,16 +1083,21 @@ static void
 write_archive (iarch)
      bfd *iarch;
 {
+  int tempfd;
   bfd *obfd;
   char *old_name, *new_name;
   bfd *contents_head = iarch->next;

   old_name = xmalloc (strlen (bfd_get_filename (iarch)) + 1);
   strcpy (old_name, bfd_get_filename (iarch));
-  new_name = make_tempname (old_name);
+  tempfd = create_tempfile (old_name, &new_name);

   output_filename = new_name;

+  /* FIXME: Why fdopenr fails in bfd_close later but openw works ok?
+     Because of that I have to explicitly close the tempfd. *sigh*.
+     obfd = bfd_fdopenr (new_name, bfd_get_target (iarch), tempfd);  */
+  close(tempfd);
   obfd = bfd_openw (new_name, bfd_get_target (iarch));

   if (obfd == NULL)


--- ../../src-cvs/binutils/bucomm.c	Thu Jul  3 00:57:25 2003
+++ bucomm.c	Thu Jul  3 11:50:20 2003
@@ -400,10 +400,10 @@ print_arelt_descr (file, abfd, verbose)
   fprintf (file, "%s\n", bfd_get_filename (abfd));
 }

-/* Return the name of a temporary file in the same directory as FILENAME.  */
+/* Return the base name of FILENAME */

 char *
-make_tempname (filename)
+get_basetemplate (filename)
      char *filename;
 {
   static char template[] = "stXXXXXX";
@@ -438,17 +438,43 @@ make_tempname (filename)
 #endif
       strcat (tmpname, "/");
       strcat (tmpname, template);
-      mktemp (tmpname);
       *slash = c;
     }
   else
     {
       tmpname = xmalloc (sizeof (template));
       strcpy (tmpname, template);
-      mktemp (tmpname);
     }
   return tmpname;
 }
+
+/* Creates a temporary file in the same directory as NAME, and returns
+   its file descriptor.  NEWNAME is set to the path of the file.
+   On failure, -1 is returned.  */
+
+int
+create_tempfile (name, newname)
+    char *name;
+    char **newname;
+{
+  *newname = get_basetemplate(name);
+  return (mkstemps(*newname, 0));
+}
+
+/* Creates a temporary directory in the same directory as NAME.
+   NEWNAME is set to the path of the diretory. On success 0 (zero)
+   is returned and on failure, -1 is returned.  */
+
+int
+create_tempdir (name, newname)
+    char *name;
+    char **newname;
+{
+
+  *newname = get_basetemplate(name);
+   return (mkdtemps(*newname, 0));
+}
+

 /* Parse a string into a VMA, with a fatal error if it can't be
    parsed.  */


--- ../../src-cvs/binutils/bucomm.h	Thu Jul  3 00:57:25 2003
+++ bucomm.h	Thu Jul  3 11:49:55 2003
@@ -181,8 +181,14 @@ int display_info
 void print_arelt_descr
   PARAMS ((FILE *, bfd *, bfd_boolean));

-char *make_tempname
+char *get_basetemplate
   PARAMS ((char *));
+
+int create_tempfile
+  PARAMS ((char *, char **));
+
+int create_tempdir
+  PARAMS ((char *, char **));

 bfd_vma parse_vma
   PARAMS ((const char *, const char *));


--- ../../src-cvs/binutils/objcopy.c	Thu Jul  3 00:57:25 2003
+++ objcopy.c	Thu Jul  3 11:58:51 2003
@@ -1523,14 +1523,15 @@ copy_archive (ibfd, obfd, output_target)
     } *list, *l;
   bfd **ptr = &obfd->archive_head;
   bfd *this_element;
-  char *dir = make_tempname (bfd_get_filename (obfd));
-
+  char *dir;
+
   /* Make a temp directory to hold the contents.  */
-  if (MKDIR (dir, 0700) != 0)
+  if (create_tempdir (bfd_get_filename (obfd), &dir) != 0)
     {
-      fatal (_("cannot mkdir %s for archive copying (error: %s)"),
-	     dir, strerror (errno));
+      fatal (_("cannot create_tempdir %s for archive copying (error: %s)"),
+            dir, strerror(errno));
     }
+
   obfd->has_armap = ibfd->has_armap;

   list = NULL;
@@ -1555,12 +1556,12 @@ copy_archive (ibfd, obfd, output_target)
       /* If the file already exists, make another temp dir.  */
       if (stat (output_name, &buf) >= 0)
 	{
-	  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));
-	    }
+          if (create_tempdir (output_name, &output_name) != 0)
+            {
+              fatal (_("cannot create_tempdir %s for archive copying (error: %s)"),
+                     output_name, strerror(errno));
+            }
+
 	  l = (struct name_list *) xmalloc (sizeof (struct name_list));
 	  l->name = output_name;
 	  l->next = list;
@@ -2348,10 +2349,15 @@ strip_main (argc, argv)
 	    }
 	}

-      if (output_file != NULL)
+      if (output_file != NULL) {
 	tmpname = output_file;
-      else
-	tmpname = make_tempname (argv[i]);
+      } else {
+        int tempfd = create_tempfile(argv[i], &tmpname);
+	/* This is ugly. but mkstemp* opens the file anyway. */
+	if (tempfd > 0)
+	 close(tempfd);
+      }
+
       status = 0;

       copy_file (argv[i], tmpname, input_target, output_target);
@@ -2898,7 +2904,11 @@ copy_main (argc, argv)
   if ((output_filename == (char *) NULL) ||
       (strcmp (input_filename, output_filename) == 0))
     {
-      char *tmpname = make_tempname (input_filename);
+      char *tmpname;
+      int tempfd = create_tempfile (input_filename, &tmpname);
+
+      if (tempfd > 0)
+      	close(tempfd);

       copy_file (input_filename, tmpname, input_target, output_target);
       if (status == 0)


--- ../../src-cvs/libiberty/configure.in	Thu Jul  3 00:57:33 2003
+++ configure.in	Thu Jul  3 10:12:12 2003
@@ -181,6 +181,7 @@ funcs="$funcs memmove"
 funcs="$funcs mempcpy"
 funcs="$funcs memset"
 funcs="$funcs mkstemps"
+funcs="$funcs mkdtemps"
 funcs="$funcs putenv"
 funcs="$funcs random"
 funcs="$funcs rename"
@@ -219,7 +220,7 @@ checkfuncs="$checkfuncs getsysinfo table
 # autoheader happy without adding a bunch of text to acconfig.h.
 if test "x" = "y"; then
   AC_CHECK_FUNCS(asprintf atexit basename bcmp bcopy bsearch bzero calloc clock \
-  getcwd getpagesize index insque mkstemps memchr memcmp memcpy \
+  getcwd getpagesize index insque mkstemps mkdtemps memchr memcmp memcpy \
   memmove mempcpy memset putenv random rename rindex sigsetmask \
   strcasecmp setenv stpcpy stpncpy strchr strdup strncasecmp strrchr strstr \
   strtod strtol strtoul tmpnam vasprintf vfprintf vprintf \

--- ../../src-cvs/libiberty/Makefile.in	Thu Jul  3 00:57:33 2003
+++ Makefile.in	Thu Jul  3 10:13:57 2003
@@ -140,7 +140,7 @@ CFILES = alloca.c argv.c asprintf.c atex
 	lrealpath.c							\
 	make-relative-prefix.c						\
 	make-temp-file.c md5.c memchr.c memcmp.c memcpy.c memmove.c	\
-	 mempcpy.c memset.c mkstemps.c					\
+	 mempcpy.c memset.c mkstemps.c mkdtemps.c			\
 	objalloc.c obstack.c						\
 	partition.c							\
 	 pex-djgpp.c pex-mpw.c pex-msdos.c pex-os2.c			\
@@ -186,7 +186,7 @@ CONFIGURED_OFILES = asprintf.o atexit.o
 	ffs.o								\
 	getcwd.o getpagesize.o						\
 	index.o insque.o						\
-	memchr.o memcmp.o memcpy.o memmove.o mempcpy.o memset.o mkstemps.o \
+	memchr.o memcmp.o memcpy.o memmove.o mempcpy.o memset.o mkstemps.o mkdtemps.o \
 	pex-djgpp.o pex-mpw.o pex-msdos.o pex-os2.o			\
 	 pex-unix.o pex-win32.o						\
 	 putenv.o							\
@@ -463,6 +463,7 @@ memmove.o: $(INCDIR)/ansidecl.h
 mempcpy.o: $(INCDIR)/ansidecl.h
 memset.o: $(INCDIR)/ansidecl.h
 mkstemps.o: config.h $(INCDIR)/ansidecl.h
+mkdtemps.o: config.h $(INCDIR)/ansidecl.h
 objalloc.o: config.h $(INCDIR)/ansidecl.h $(INCDIR)/objalloc.h
 obstack.o: config.h $(INCDIR)/obstack.h
 partition.o: config.h $(INCDIR)/ansidecl.h $(INCDIR)/libiberty.h \


---------------------------FILE mkdtemps.c-------------------------------
/* Copyright (C) 1991, 1992, 1996, 1998 Free Software Foundation, Inc.
   This file is derived from mkstemp.c from the GNU C Library.

   The GNU C Library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Library General Public License as
   published by the Free Software Foundation; either version 2 of the
   License, or (at your option) any later version.

   The GNU C Library is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
   Library General Public License for more details.

   You should have received a copy of the GNU Library General Public
   License along with the GNU C Library; see the file COPYING.LIB.  If not,
   write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
   Boston, MA 02111-1307, USA.  */

#ifdef HAVE_CONFIG_H
#include "config.h"
#endif

#include <sys/types.h>
#include <sys/stat.h>
#ifdef HAVE_STDLIB_H
#include <stdlib.h>
#endif
#ifdef HAVE_STRING_H
#include <string.h>
#endif
#include <errno.h>
#include <stdio.h>
#include <fcntl.h>
#ifdef HAVE_UNISTD_H
#include <unistd.h>
#endif
#ifdef HAVE_SYS_TIME_H
#include <sys/time.h>
#endif
#include "ansidecl.h"

/* We need to provide a type for gcc_uint64_t.  */
#ifdef __GNUC__
__extension__ typedef unsigned long long gcc_uint64_t;
#else
typedef unsigned long gcc_uint64_t;
#endif

#ifndef TMP_MAX
#define TMP_MAX 16384
#endif

#undef MKDIR
#if defined (_WIN32) && !defined (__CYGWIN32__)
#define MKDIR(DIR, MODE) mkdir (DIR)
#else
#define MKDIR(DIR, MODE) mkdir (DIR, MODE)
#endif

/*

@deftypefn Replacement int mkdtemps (char *@var{template}, int @var{suffix_len})

Generate a unique temporary file name from @var{template}.
@var{template} has the form:

@example
   @var{path}/ccXXXXXX@var{suffix}
@end example

@var{suffix_len} tells us how long @var{suffix} is (it can be zero
length).  The last six characters of @var{template} before @var{suffix}
must be @samp{XXXXXX}; they are replaced with a string that makes the
filename unique.  Returns a file descriptor open on the file for
reading and writing.

@end deftypefn

*/

int
mkdtemps (template, suffix_len)
     char *template;
     int suffix_len;
{
  static const char letters[]
    = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
  static gcc_uint64_t value;
#ifdef HAVE_GETTIMEOFDAY
  struct timeval tv;
#endif
  char *XXXXXX;
  size_t len;
  int count;

  len = strlen (template);

  if ((int) len < 6 + suffix_len
      || strncmp (&template[len - 6 - suffix_len], "XXXXXX", 6))
    {
      return -1;
    }

  XXXXXX = &template[len - 6 - suffix_len];

#ifdef HAVE_GETTIMEOFDAY
  /* Get some more or less random data.  */
  gettimeofday (&tv, NULL);
  value += ((gcc_uint64_t) tv.tv_usec << 16) ^ tv.tv_sec ^ getpid ();
#else
  value += getpid ();
#endif

  for (count = 0; count < TMP_MAX; ++count)
    {
      gcc_uint64_t v = value;
      int ret_val;

      /* Fill in the random bits.  */
      XXXXXX[0] = letters[v % 62];
      v /= 62;
      XXXXXX[1] = letters[v % 62];
      v /= 62;
      XXXXXX[2] = letters[v % 62];
      v /= 62;
      XXXXXX[3] = letters[v % 62];
      v /= 62;
      XXXXXX[4] = letters[v % 62];
      v /= 62;
      XXXXXX[5] = letters[v % 62];

      ret_val = MKDIR(template, 0700);

      if (ret_val == 0)
	/* The file does not exist.  */
	return ret_val;

      /* This is a random value.  It is only necessary that the next
	 TMP_MAX values generated by adding 7777 to VALUE are different
	 with (module 2^32).  */
      value += 7777;
    }

  /* We return the null string if we can't find a unique file name.  */
  template[0] = '\0';
  return -1;
}