Patch to reduce usage of tempnam()

[Pavel Roskin <proski gnu org>]

Hello!

This patch should make it easier to work with temporary files - it adds a
new function mc_mkstemps(), similar to mkstemps() from libibery and C
library on OpenBSD.

The difference is that mc_mkstemps creates the template for you and even
adds the directory name (/tmp or $TMPDIR if set). It can be changed to
call the native mkstemps if it's available.

Don't forget that tempnam() is unsafe on some systems, and GNU ld even
emits a warning about it. Unfortunately, the warning is not fixed by this
patch, since the editor uses tempnam() in some non-trivial way.

There are many places in the code where the results of tempnam() and the
subsequent open() are handled inconsistently. Now it's possible to use one
function instead of two, thus the error processing becomes easier.

mc_mkstemps() uses g_strconcat() thus making g_tempnam() useless. The
later is removed by this patch.

Support for suffixes makes it easier to work with programs and OS'es that
require them. It should be easy to fix e.g. rpm support on remote
filesystems - just add the suffix of the remote file. It's not done, but
it will be easier to do with this patch.

SCRIPT_SUFFIX is defined to ".cmd" on OS/2 and Windows NT. It's used on
temporary files that are later executed.

Please review the patch. I'm not applying it yet.

ChangeLog:
	* src/ext.c (exec_extension): Use mc_mkstemps().
	* src/main.c (main): Call init_tmpdir().
	* src/user.c (execute_menu_command): Use mc_mkstemps().
	* src/util.c (init_tmpdir): New function - initialize temp
	directory.
	(mc_mkstemps): New function - safely create and open temporary
	file.
	* src/util.h: Declarations for init_tmpdir() and mc_mkstemps().
	Define TMPDIR_DEFAULT and SCRIPT_SUFFIX.
	* vfs/direntry.c (vfs_s_open): Use mc_mkstemps. Create and close
	temporary file to reserve its name on the filesystem.
	(vfs_s_retrieve_file): Use mc_mkstemps().
	(g_tempnam): Remove.
	* vfs/extfs.c (extfs_open): Use mc_mkstemps().
	* vfs/sfs.c (redirect): Likewise.
	* vfs/shared_ftp_fish.c (_get_file_entry): Likewise.
	(retrieve_file): Likewise.
	* vfs/vfs.c (mc_def_getlocalcopy): Likewise.
	* vfs/xdirentry.h: Remove declaration of g_tempnam().

-- 
Regards,
Pavel Roskin

___________________________________________________________________
--- src/ext.c
+++ src/ext.c
@@ -139,14 +139,10 @@ exec_extension (const char *filename, co
     else
 	do_local_copy = 0;

-    if ((file_name = tempnam (NULL, "mcext")) == 0) {
-	message (1, MSG_ERROR, _(" Can't generate unique filename \n %s "),
-		 unix_error_string (errno));
-	return;
-    }
+    cmd_file_fd = mc_mkstemps(&file_name, "mcext", SCRIPT_SUFFIX);

     /* #warning FIXME: this is ugly */
-    if ((cmd_file_fd = open (file_name, O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600)) == -1){
+    if (cmd_file_fd == -1){
 	message (1, MSG_ERROR, _(" Can't create temporary command file \n %s "),
 		 unix_error_string (errno));
 	free (file_name);
--- src/main.c
+++ src/main.c
@@ -3013,7 +3013,10 @@ main (int argc, char *argv [])
 #endif
     /* Initialize list of all user group for timur_clr_mode */
     init_groups ();
-
+
+    /* Initialize temporary directory */
+    init_tmpdir ();
+
     OS_Setup ();

     /* This variable is used by the subshell */
--- src/user.c
+++ src/user.c
@@ -540,19 +540,9 @@ execute_menu_command (char *commands)
 	return;
     }

-    if ((file_name = tempnam (NULL, "mcusr")) == 0) {
-	message (1, MSG_ERROR, _(" Can't generate unique filename \n %s "),
-		 unix_error_string (errno));
-	return;
-    }
+    cmd_file_fd = mc_mkstemps(&file_name, "mcusr", SCRIPT_SUFFIX);

-#ifdef OS2_NT
-    /* OS/2 and NT requires the command to end in .cmd */
-    p = file_name;
-    file_name = g_strconcat (file_name, ".cmd", NULL);
-    free (p);
-#endif
-    if ((cmd_file_fd = open (file_name, O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600)) == -1){
+    if (cmd_file_fd == -1){
 	message (1, MSG_ERROR, _(" Can't create temporary command file \n %s "),
 		 unix_error_string (errno));
         free (file_name);
--- src/util.c
+++ src/util.c
@@ -86,6 +86,9 @@ int tilde_trunc = 1;

 struct mount_entry *mount_list = NULL;

+/* temporary directory, g_malloc'ed, ends with PATH_SEP */
+static char* mc_tmpdir;
+
 #ifndef VFS_STANDALONE
 int is_printable (int c)
 {
@@ -1272,3 +1275,97 @@ concat_dir_and_file (const char *dir, co
 	return  g_strconcat (dir, PATH_SEP_STR, file, NULL);
 }

+/* Initialize temporary directory */
+void init_tmpdir(void)
+{
+    char *p;
+
+    p = getenv("TMPDIR");
+    if (!p) {
+	p = TMPDIR_DEFAULT;
+    }
+
+    /* Allocate and ensure that is ends with PATH_SEP */
+    mc_tmpdir = concat_dir_and_file (p, "");
+}
+
+/* Following code heavily borrows from libiberty, mkstemps.c */
+
+#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
+
+/*
+ * Arguments:
+ * pname - if not NULL, put the filename here.
+ * prefix - filename only, temporary directory is prepended.
+ * suffix - if not NULL, appended after the random part.
+ *
+ * Result:
+ * handle of the open file or -1 if couldn't open any.
+ */
+int mc_mkstemps(char **pname, const char *prefix, const char *suffix)
+{
+    static const char letters[]
+	= "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+    static gcc_uint64_t value;
+    struct timeval tv;
+    char *tmpname;
+    char *XXXXXX;
+    int count;
+
+    tmpname = g_strconcat (mc_tmpdir, prefix, "XXXXXX", suffix, NULL);
+    if (pname)
+	*pname = tmpname;
+    XXXXXX = &tmpname[strlen (mc_tmpdir) + strlen (prefix)];
+
+    /* Get some more or less random data.  */
+    gettimeofday (&tv, NULL);
+    value += (tv.tv_usec << 16) ^ tv.tv_sec ^ getpid ();
+
+    for (count = 0; count < TMP_MAX; ++count) {
+	gcc_uint64_t v = value;
+	int fd;
+
+	/* 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];
+
+	fd = open (tmpname, O_RDWR|O_CREAT|O_EXCL, 0600);
+	if (fd >= 0) {
+	    /* Successfully created.  */
+	    if (!pname)
+		g_free (tmpname);
+	    return fd;
+	}
+
+	/* 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.
+       Of course, only if the caller wants any string.  */
+    if (!pname)
+	g_free (tmpname);
+    else
+	tmpname[0] = '\0';
+
+    return -1;
+}
--- src/util.h
+++ src/util.h
@@ -146,6 +146,10 @@

 extern char app_text [];

+/* Creating temporary files safely */
+void init_tmpdir(void);
+int mc_mkstemps(char **pname, const char *prefix, const char *suffix);
+
 enum {
 	ISGUNZIPABLE_GUNZIP,
 	ISGUNZIPABLE_BZIP,
@@ -179,6 +183,8 @@
 #    define PATH_SEP '\\'
 #    define PATH_SEP_STR "\\"
 #    define PATH_ENV_SEP ';'
+#    define TMPDIR_DEFAULT "c:\\temp"
+#    define SCRIPT_SUFFIX ".cmd"
 #    define OS_SORT_CASE_SENSITIVE_DEFAULT 0
 #    define STRCOMP stricmp
 #    define STRNCOMP strnicmp
@@ -190,6 +196,8 @@
 #    define PATH_SEP '/'
 #    define PATH_SEP_STR "/"
 #    define PATH_ENV_SEP ':'
+#    define TMPDIR_DEFAULT "/tmp"
+#    define SCRIPT_SUFFIX ""
 #    define get_default_editor() "vi"
 #    define OS_SORT_CASE_SENSITIVE_DEFAULT 1
 #    define STRCOMP strcmp
--- vfs/direntry.c
+++ vfs/direntry.c
@@ -754,6 +754,7 @@ vfs_s_open (vfs *me, char *file, int fla
 	char *dirname, *name, *save;
 	vfs_s_entry *ent;
 	vfs_s_inode *dir;
+	int tmp_handle;
 	if (!(flags & O_CREAT))
 	    return NULL;

@@ -766,7 +767,10 @@ vfs_s_open (vfs *me, char *file, int fla
 	ent = vfs_s_generate_entry (me, name, dir, 0755);
 	ino = ent->ino;
 	vfs_s_insert_entry (me, dir, ent);
-	ino->localname = g_tempnam (NULL, me->name);
+	tmp_handle = mc_mkstemps (&ino->localname, me->name, NULL);
+	if (tmp_handle == -1)
+	    return NULL;
+	close (tmp_handle);
 	was_changed = 1;
     }

@@ -935,9 +939,8 @@ vfs_s_retrieve_file(vfs *me, struct vfs_
     memset(&fh, 0, sizeof(fh));

     fh.ino = ino;
-    if (!(ino->localname = g_tempnam (NULL, me->name))) ERRNOR (ENOMEM, 0);

-    handle = open(ino->localname, O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600);
+    handle = mc_mkstemps (&ino->localname, me->name, NULL);
     if (handle == -1) {
 	me->verrno = errno;
 	goto error_4;
@@ -1185,20 +1188,3 @@ vfs_s_get_line_interruptible (vfs *me, c
     buffer [size-1] = 0;
     return 0;
 }
-
-
-/* Roland: on most non-GNU/Linux platforms malloc()!=g_malloc() which
- * may cause crashes. This wrapper ensures that memory from tempnam
- * can safely free'ed with g_free()
- */
-char *
-g_tempnam( const char *dir, const char *prefix )
-{
-    char *tmp = (char *)tempnam( dir, prefix );
-    char *name;
-
-    name = g_strdup( tmp );
-    free( tmp );
-    return( name );
-}
-
--- vfs/extfs.c
+++ vfs/extfs.c
@@ -603,11 +603,10 @@ static void *extfs_open (vfs *me, char *
         char *cmd;
 	char *archive_name, *p;

-        entry->inode->local_filename = g_tempnam (NULL, "extfs");
 	{
 	    int handle;
+	    handle = mc_mkstemps (&entry->inode->local_filename, "extfs", NULL);

-	    handle = open(entry->inode->local_filename, O_RDWR | O_CREAT | O_EXCL, 0600);
 	    if (handle == -1)
 		return NULL;
 	    close(handle);
--- vfs/sfs.c
+++ vfs/sfs.c
@@ -126,8 +126,8 @@ redirect (vfs *me, char *name)
 	cur = cur->next;
     }

-    cache = tempnam (NULL, "sfs");
-    handle = open (cache, O_RDWR | O_CREAT | O_EXCL, 0600);
+    handle = mc_mkstemps (&cache, "sfs", NULL);
+
     if (handle == -1) {
 	g_free (cache);
 	return "/SOMEONE_PLAYING_DIRTY_TMP_TRICKS_ON_US";
--- vfs/shared_ftp_fish.c
+++ vfs/shared_ftp_fish.c
@@ -330,9 +330,7 @@ _get_file_entry(struct connection *bucke
 			ent->local_filename = NULL;
 		    }
 		    if (flags & O_TRUNC) {
-			ent->local_filename = g_tempnam (NULL, X "fs");
-			if (ent->local_filename == NULL) ERRNOR (ENOMEM, NULL);
-			handle = open(ent->local_filename, O_CREAT | O_TRUNC | O_RDWR | O_EXCL, 0600);
+			handle = mc_mkstemps (&ent->local_filename, X "fs", NULL);
 			if (handle < 0) ERRNOR (EIO, NULL);
 			close(handle);
 			if (stat (ent->local_filename, &ent->local_stat) < 0)
@@ -368,12 +366,8 @@ _get_file_entry(struct connection *bucke
 	ent->bucket = bucket;
 	ent->name = g_strdup(p);
 	ent->remote_filename = g_strdup(file_name);
-	ent->local_filename = g_tempnam (NULL, X "fs");
-	if (!ent->name || !ent->remote_filename || !ent->local_filename) {
-	    direntry_destructor(ent);
-	    ERRNOR (ENOMEM, NULL);
-	}
-        handle = open (ent->local_filename, O_CREAT | O_EXCL | O_RDWR | O_TRUNC, 0700);
+
+	handle = mc_mkstemps (&ent->local_filename, X "fs", NULL);
 	if (handle == -1) {
 	    my_errno = EIO;
 	    goto error;
@@ -841,10 +835,9 @@ static int retrieve_file(struct direntry

     if (fe->local_filename)
         return 1;
-    if (!(fe->local_filename = g_tempnam (NULL, X))) ERRNOR (ENOMEM, 0);
     fe->local_is_temp = 1;

-    local_handle = open(fe->local_filename, O_RDWR | O_CREAT | O_TRUNC | O_EXCL, 0600);
+    local_handle = mc_mkstemps (&fe->local_filename, X, NULL);
     if (local_handle == -1) {
 	my_errno = EIO;
 	goto error_4;
--- vfs/vfs.c
+++ vfs/vfs.c
@@ -1062,8 +1062,8 @@ mc_def_getlocalcopy (vfs *vfs, char *fil
     fdin = mc_open (filename, O_RDONLY);
     if (fdin == -1)
         return NULL;
-    tmp = g_tempnam (NULL, "mclocalcopy");
-    fdout = open (tmp, O_CREAT|O_WRONLY|O_TRUNC|O_EXCL, 0600);
+
+    fdout = mc_mkstemps (&tmp, "mclocalcopy", NULL);
     if (fdout == -1)
 	goto fail;
     while ((i = mc_read (fdin, buffer, sizeof (buffer))) > 0){
--- vfs/xdirentry.h
+++ vfs/xdirentry.h
@@ -254,8 +254,6 @@

 /* misc */
 int vfs_s_retrieve_file (vfs *me, struct vfs_s_inode *ino);
-/* alloc temp name which can be safely free'd with g_free() */
-char *g_tempnam( const char *dir, const char *prefix );

 #if 0
 /* If non-null, FREE */
___________________________________________________________________