1. added wxFileName::CreateTempFileName() and implemented it properly (using

mkstemp() when available)
2. wxTempFile::Open() and wxGetTempFileName() now use CreateTempFileName()
   avoiding code duplication
3. updated the docs


git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@12805 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

configure.in

@@ -3055,6 +3055,9 @@ fi
 dnl check for vfork() (even if it's the same as fork() in modern Unices)
 AC_CHECK_FUNCS(vfork)
 
+dnl check for the function for temp files creation
+AC_CHECK_FUNCS(mkstemp mktemp, break)
+
 dnl get the library function to use for wxGetDiskSpace(): it is statfs() under
 dnl Linux and *BSD and statvfs() under Solaris
 AC_CACHE_CHECK(for statfs, wx_cv_func_statfs,

docs/latex/wx/filename.tex

@@ -234,15 +234,33 @@ get the value of user home (Unix only mainly)
 
 \func{void}{AssignTempFileName}{\param{const wxString\& }{prefix}}
 
-get a temp file name starting with thespecified prefix
+The function calls \helpref{CreateTempFileName}{wxfilenamecreatetempfilename} to
-
+create a temporary file and sets this object to the name of the file. If a
+temporary file couldn't be created, the object is put into the\rtfsp
+\helpref{invalid}{wxfilenameisok} state.
 
 \membersection{wxFileName::Clear}\label{wxfilenameclear}
 
 \func{void}{Clear}{\void}
 
-reset all components to default, uninitialized state
+Reset all components to default, uninitialized state.
 
+\membersection{wxFileName::CreateTempFileName}{wxfilenamecreatetempfilename}
+
+\func{static wxString}{CreateTempFileName}{\param{const wxString\& }{prefix}}
+
+Returns a temporary file name starting with the given {\it prefix}. If
+the {\it prefix} is an absolute path, the temporary file is created in this
+directory, otherwise it is created in the default system directory for the
+temporary files or in the current directory.
+
+If the function succeeds, the temporary file is actually created (but not
+opened) as well. Under Unix, it will have read and write permissions for the
+owner only.
+
+\wxheading{Return value}
+
+The full temporary file name or an empty string on error.
 
 \membersection{wxFileName::DirExists}\label{wxfilenamedirexists}
 
@@ -443,9 +461,10 @@ are the file names of this type cases sensitive?
 
 \constfunc{bool}{IsOk}{\void}
 
-file tests
+Returns {\tt TRUE} if the filename is valid, {\tt FALSE} if it is not
-is the filename valid at all?
+initialized yet. The assignment functions and
-
+\helpref{Clear}{wxfilenameclear} may reset the object to the uninitialized,
+invalid state (the former only do it on failure).
 
 \membersection{wxFileName::IsPathSeparator}\label{wxfilenameispathseparator}
 

docs/latex/wx/function.tex

@@ -265,15 +265,19 @@ if the buffer is NULL.
 
 \func{bool}{wxGetTempFileName}{\param{const wxString\& }{prefix}, \param{wxString\& }{buf}}
 
-Makes a temporary filename based on {\it prefix}, opens and closes the file,
+%% Makes a temporary filename based on {\it prefix}, opens and closes the file,
-and places the name in {\it buf}. If {\it buf} is NULL, new store
+%% and places the name in {\it buf}. If {\it buf} is NULL, new store
-is allocated for the temporary filename using {\it new}.
+%% is allocated for the temporary filename using {\it new}.
+%%
+%% Under Windows, the filename will include the drive and name of the
+%% directory allocated for temporary files (usually the contents of the
+%% TEMP variable). Under Unix, the {\tt /tmp} directory is used.
+%%
+%% It is the application's responsibility to create and delete the file.
 
-Under Windows, the filename will include the drive and name of the
+These functions are obsolete, please use\rtfsp
-directory allocated for temporary files (usually the contents of the
+\helpref{wxFileName::CreateTempFileName}{wxfilenamecreatetempfilename}\rtfsp
-TEMP variable). Under Unix, the {\tt /tmp} directory is used.
+instead.
-
-It is the application's responsibility to create and delete the file.
 
 \membersection{::wxIsWild}\label{wxiswild}
 

include/wx/filefn.h

@@ -185,7 +185,7 @@ WXDLLEXPORT wxString wxUnix2MacFilename( const char *s);
 WXDLLEXPORT void wxStripExtension(wxChar *buffer);
 WXDLLEXPORT void wxStripExtension(wxString& buffer);
 
-// Get a temporary filename, opening and closing the file.
+// Get a temporary filename
 WXDLLEXPORT wxChar* wxGetTempFileName(const wxString& prefix, wxChar *buf = (wxChar *) NULL);
 WXDLLEXPORT bool wxGetTempFileName(const wxString& prefix, wxString& buf);
 

include/wx/filename.h

@@ -202,6 +202,7 @@ public:
 
         // get a temp file name starting with the specified prefix
     void AssignTempFileName(const wxString& prefix);
+    static wxString CreateTempFileName(const wxString& prefix);
 
     // directory creation and removal.
     // if full is TRUE, will try to make each directory in the path.

samples/console/console.cpp

@@ -844,6 +844,35 @@ static void TestFileNameSplit()
     }
 }
 
+static void TestFileNameTemp()
+{
+    puts("*** testing wxFileName temp file creation ***");
+
+    static const char *tmpprefixes[] =
+    {
+        "foo",
+        "/tmp/foo",
+        "..",
+        "../bar",
+        "/tmp/foo/bar", // this one must be an error
+    };
+
+    for ( size_t n = 0; n < WXSIZEOF(tmpprefixes); n++ )
+    {
+        wxString path = wxFileName::CreateTempFileName(tmpprefixes[n]);
+        if ( !path.empty() )
+        {
+            printf("Prefix '%s'\t-> temp file '%s'\n",
+                   tmpprefixes[n], path.c_str());
+
+            if ( !wxRemoveFile(path) )
+            {
+                wxLogWarning("Failed to remove temp file '%s'", path.c_str());
+            }
+        }
+    }
+}
+
 static void TestFileNameComparison()
 {
     // TODO!
@@ -5187,10 +5216,11 @@ int main(int argc, char **argv)
 #endif // TEST_FILE
 
 #ifdef TEST_FILENAME
-    TestFileNameConstruction();
+    TestFileNameTemp();
-    TestFileNameSplit();
     if ( 0 )
     {
+    TestFileNameConstruction();
+    TestFileNameSplit();
         TestFileNameCwd();
         TestFileNameComparison();
         TestFileNameOperations();

setup.h.in

@@ -964,6 +964,12 @@
 /* Define if you have the inet_aton function.  */
 #undef HAVE_INET_ATON
 
+/* Define if you have the mktemp function.  */
+#undef HAVE_MKTEMP
+
+/* Define if you have the mkstemp function.  */
+#undef HAVE_MKSTEMP
+
 /* Define if you have the nanosleep function.  */
 #undef HAVE_NANOSLEEP
 

src/common/file.cpp

@@ -54,7 +54,6 @@
     #define   NOMCX
 #endif
 
-    #include  <windows.h>     // for GetTempFileName
 #elif (defined(__UNIX__) || defined(__GNUWIN32__))
     #include  <unistd.h>
     #ifdef __GNUWIN32__
@@ -121,10 +120,14 @@
 #endif // Salford C
 
 // wxWindows
+#ifndef WX_PRECOMP
     #include  "wx/string.h"
     #include  "wx/intl.h"
     #include  "wx/file.h"
     #include  "wx/log.h"
+#endif // !WX_PRECOMP
+
+#include  "wx/filename.h"
 
 // ============================================================================
 // implementation of wxFile
@@ -465,87 +468,39 @@ bool wxTempFile::Open(const wxString& strName)
 {
     m_strName = strName;
 
-    // we want to create the file in the same directory as strName because
+    m_strTemp = wxFileName::CreateTempFileName(strName);
-    // otherwise rename() in Commit() might not work (if the files are on
+
-    // different partitions for example). Unfortunately, the only standard
+    if ( m_strTemp.empty() )
-    // (POSIX) temp file creation function tmpnam() can't do it.
+    {
-#if defined(__UNIX__) || defined(__WXSTUBS__)|| defined( __WXMAC__ )
+        // CreateTempFileName() failed
-    static const wxChar *szMktempSuffix = wxT("XXXXXX");
+        return FALSE;
-    m_strTemp << strName << szMktempSuffix;
+    }
-    // can use the cast because length doesn't change
-    mktemp(wxMBSTRINGCAST m_strTemp.mb_str());
-#elif  defined(__WXPM__)
-    // for now just create a file
-    // future enhancements can be to set some extended attributes for file systems
-    // OS/2 supports that have them (HPFS, FAT32) and security (HPFS386)
-    static const wxChar *szMktempSuffix = wxT("XXX");
-    m_strTemp << strName << szMktempSuffix;
-    // Temporarily remove - MN
-    #ifndef __WATCOMC__
-    ::DosCreateDir(m_strTemp.GetWriteBuf(MAX_PATH), NULL);
-    #endif
-#else // Windows
-    wxString strPath;
-    wxSplitPath(strName, &strPath, NULL, NULL);
-    if ( strPath.IsEmpty() )
-        strPath = wxT('.');  // GetTempFileName will fail if we give it empty string
-#ifdef __WIN32__
-    if ( !GetTempFileName(strPath, wxT("wx_"),0, m_strTemp.GetWriteBuf(MAX_PATH)) )
-#else
-        // Not sure why MSVC++ 1.5 header defines first param as BYTE - bug?
-        if ( !GetTempFileName((BYTE) (DWORD)(const wxChar*) strPath, wxT("wx_"),0, m_strTemp.GetWriteBuf(MAX_PATH)) )
-#endif
-            wxLogLastError(wxT("GetTempFileName"));
-    m_strTemp.UngetWriteBuf();
-#endif  // Windows/Unix
 
-    int access = wxS_DEFAULT;
 #ifdef __UNIX__
-    // create the file with the same mode as the original one under Unix
+    // the temp file should have the same permissions as the original one
-    mode_t umaskOld = 0; // just to suppress compiler warning
+    mode_t mode;
-    bool changedUmask;
 
     wxStructStat st;
     if ( stat(strName.fn_str(), &st) == 0 )
     {
-        // this assumes that only lower bits of st_mode contain the access
+        mode = st.st_mode;
-        // rights, but it's true for at least all Unices which have S_IXXXX()
-        // macros, so should not be less portable than using (not POSIX)
-        // S_IFREG &c
-        access = st.st_mode & 0777;
-
-        // we want to create the file with exactly the same access rights as
-        // the original one, so disable the user's umask for the moment
-        umaskOld = umask(0);
-        changedUmask = TRUE;
     }
     else
     {
-        // file probably didn't exist, just create with default mode _using_
+        // file probably didn't exist, just give it the default mode _using_
         // user's umask (new files creation should respect umask)
-        changedUmask = FALSE;
+        mode_t mask = umask(0777);
+        mode = 0666 & ~mask;
+        umask(mask);
     }
-#endif // Unix
 
-    // Open this file securely, since it surely should not exist unless
+    if ( chmod(m_strTemp.mb_str(), mode) == -1 )
-    // nefarious activities (or other random bad things) are at play.
-
-    bool ok = m_file.Open(m_strTemp, wxFile::write_excl, access);
-
-    // FIXME: If !ok here should we loop and try again with another file
-    //        name?  That is the standard recourse if open(O_EXCL) fails,
-    //        though of course it should be protected against possible
-    //        infinite looping too.
-
-#ifdef __UNIX__
-    if ( changedUmask )
     {
-        // restore umask now that the file is created
+        wxLogSysError(_("Failed to set temporary file permissions"));
-        (void)umask(umaskOld);
     }
 #endif // Unix
 
-    return ok;
+    return TRUE;
 }
 
 // ----------------------------------------------------------------------------
@@ -582,5 +537,5 @@ void wxTempFile::Discard()
         wxLogSysError(_("can't remove temporary file '%s'"), m_strTemp.c_str());
 }
 
-#endif
+#endif // wxUSE_FILE
 

src/common/filefn.cpp

@@ -1262,59 +1262,23 @@ bool wxPathExists(const wxChar *pszPathName)
 // Get a temporary filename, opening and closing the file.
 wxChar *wxGetTempFileName(const wxString& prefix, wxChar *buf)
 {
-#if defined(__WINDOWS__) && !defined(__WXMICROWIN__)
+    wxString filename = wxFileName::CreateTempFileName(prefix);
+    if ( filename.empty() )
+        return NULL;
 
-#ifndef        __WIN32__
-  wxChar tmp[144];
-  ::GetTempFileName(0, WXSTRINGCAST prefix, 0, tmp);
-#else
-  wxChar tmp[MAX_PATH];
-  wxChar tmpPath[MAX_PATH];
-  ::GetTempPath(MAX_PATH, tmpPath);
-  ::GetTempFileName(tmpPath, WXSTRINGCAST prefix, 0, tmp);
-#endif
-  if (buf) wxStrcpy(buf, tmp);
-  else buf = copystring(tmp);
-  return buf;
-
-#else
-  static short last_temp = 0;        // cache last to speed things a bit
-  // At most 1000 temp files to a process! We use a ring count.
-  wxChar tmp[100]; // FIXME static buffer
-
-  for (short suffix = last_temp + 1; suffix != last_temp; ++suffix %= 1000)
-    {
-      wxSprintf (tmp, wxT("/tmp/%s%d.%03x"), WXSTRINGCAST prefix, (int) getpid (), (int) suffix);
-      if (!wxFileExists( tmp ))
-        {
-          // Touch the file to create it (reserve name)
-          FILE *fd = fopen (wxFNCONV(tmp), "w");
-          if (fd)
-            fclose (fd);
-          last_temp = suffix;
     if ( buf )
-            wxStrcpy( buf, tmp);
+        wxStrcpy(buf, filename);
     else
-            buf = copystring( tmp );
+        buf = copystring(filename);
+
     return buf;
 }
-    }
-  wxLogError( _("wxWindows: error finding temporary file name.\n") );
-  if (buf) buf[0] = 0;
-  return (wxChar *) NULL;
-#endif
-}
 
 bool wxGetTempFileName(const wxString& prefix, wxString& buf)
 {
-    wxChar buf2[512];
+    buf = wxFileName::CreateTempFileName(prefix);
-    if (wxGetTempFileName(prefix, buf2) != (wxChar*) NULL)
+
-    {
+    return !buf.empty();
-        buf = buf2;
-        return TRUE;
-    }
-    else
-        return FALSE;
 }
 
 // Get first file name matching given wild card.

src/common/filename.cpp

@@ -362,15 +362,168 @@ wxString wxFileName::GetHomeDir()
 
 void wxFileName::AssignTempFileName( const wxString& prefix )
 {
-    wxString fullname;
+    wxString tempname = CreateTempFileName(prefix);
-    if ( wxGetTempFileName(prefix, fullname) )
+    if ( tempname.empty() )
-    {
-        Assign(fullname);
-    }
-    else // error
     {
+        // error, failed to get temp file name
         Clear();
     }
+    else // ok
+    {
+        Assign(tempname);
+    }
+}
+
+/* static */
+wxString wxFileName::CreateTempFileName(const wxString& prefix)
+{
+    wxString path, dir, name;
+
+    // use the directory specified by the prefix
+    SplitPath(prefix, &dir, &name, NULL /* extension */);
+
+#if defined(__WINDOWS__) && !defined(__WXMICROWIN__)
+
+#ifdef __WIN32__
+    if ( dir.empty() )
+    {
+        if ( !::GetTempPath(MAX_PATH, wxStringBuffer(dir, MAX_PATH + 1)) )
+        {
+            wxLogLastError(_T("GetTempPath"));
+        }
+
+        if ( dir.empty() )
+        {
+            // GetTempFileName() fails if we pass it an empty string
+            dir = _T('.');
+        }
+    }
+
+    if ( !::GetTempFileName(dir, name, 0, wxStringBuffer(path, MAX_PATH + 1)) )
+    {
+        wxLogLastError(_T("GetTempFileName"));
+
+        path.clear();
+    }
+#else // Win16
+    if ( !::GetTempFileName(NULL, prefix, 0, wxStringBuffer(path, 1025)) )
+    {
+        path.clear();
+    }
+#endif // Win32/16
+
+#elif defined(__WXPM__)
+    // for now just create a file
+    //
+    // future enhancements can be to set some extended attributes for file
+    // systems OS/2 supports that have them (HPFS, FAT32) and security
+    // (HPFS386)
+    static const wxChar *szMktempSuffix = wxT("XXX");
+    path << dir << _T('/') << name << szMktempSuffix;
+
+    // Temporarily remove - MN
+    #ifndef __WATCOMC__
+        ::DosCreateDir(wxStringBuffer(MAX_PATH), NULL);
+    #endif
+
+#else // !Windows, !OS/2
+    if ( dir.empty() )
+    {
+        dir = wxGetenv(_T("TMP"));
+        if ( path.empty() )
+        {
+            dir = wxGetenv(_T("TEMP"));
+        }
+
+        if ( dir.empty() )
+        {
+            // default
+            dir = _T("/tmp");
+        }
+    }
+
+    path = dir;
+
+    if ( !wxEndsWithPathSeparator(dir) &&
+            (name.empty() || !wxIsPathSeparator(name[0u])) )
+    {
+        path += _T('/');
+    }
+
+    path += name;
+
+#ifdef HAVE_MKSTEMP
+    // scratch space for mkstemp()
+    path += _T("XXXXXX");
+
+    // can use the cast here because the length doesn't change and the string
+    // is not shared
+    if ( mkstemp((char *)path.mb_str()) == -1 )
+    {
+        // this might be not necessary as mkstemp() on most systems should have
+        // already done it but it doesn't hurt neither...
+        path.clear();
+    }
+    //else: file already created
+#else // !HAVE_MKSTEMP
+
+#ifdef HAVE_MKTEMP
+    // same as above
+    path += _T("XXXXXX");
+
+    if ( !mktemp((char *)path.mb_str()) )
+    {
+        path.clear();
+    }
+#else // !HAVE_MKTEMP
+    // generate the unique file name ourselves
+    path << (unsigned int)getpid();
+
+    wxString pathTry;
+
+    static const size_t numTries = 1000;
+    for ( size_t n = 0; n < numTries; n++ )
+    {
+        // 3 hex digits is enough for numTries == 1000 < 4096
+        pathTry = path + wxString::Format(_T("%.03x"), n);
+        if ( !wxFile::Exists(pathTry) )
+        {
+            break;
+        }
+
+        pathTry.clear();
+    }
+
+    path = pathTry;
+#endif // HAVE_MKTEMP/!HAVE_MKTEMP
+
+    if ( !path.empty() )
+    {
+        // create the file - of course, there is a race condition here, this is
+        // why we always prefer using mkstemp()...
+        wxFile file;
+        if ( !file.Open(path, wxFile::write_excl, wxS_IRUSR | wxS_IWUSR) )
+        {
+            // FIXME: If !ok here should we loop and try again with another
+            //        file name?  That is the standard recourse if open(O_EXCL)
+            //        fails, though of course it should be protected against
+            //        possible infinite looping too.
+
+            wxLogError(_("Failed to open temporary file."));
+
+            path.clear();
+        }
+    }
+#endif // HAVE_MKSTEMP/!HAVE_MKSTEMP
+
+#endif // Windows/!Windows
+
+    if ( path.empty() )
+    {
+        wxLogSysError(_("Failed to create a temporary file name"));
+    }
+
+    return path;
 }
 
 // ----------------------------------------------------------------------------