avoid needless Unicode<->MB conversions in Unix wxExecute(); simplify the code; provide both versions taking char** and wchar_t** for compatibility; also use wxMacExecute() (renamed to wxMacLaunch() to avoid confusion) from all wxExecute() overloads but don't use it if wxEXEC_SYNC was requested as it doesn't support it

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

include/wx/utils.h

@@ -347,10 +347,15 @@ enum
 // If flags contain wxEXEC_SYNC, return -1 on failure and the exit code of the
 // process if everything was ok. Otherwise (i.e. if wxEXEC_ASYNC), return 0 on
 // failure and the PID of the launched process if ok.
-WXDLLIMPEXP_BASE long wxExecute(wxChar **argv, int flags = wxEXEC_ASYNC,
+WXDLLIMPEXP_BASE long wxExecute(wchar_t **argv,
-                           wxProcess *process = (wxProcess *) NULL);
+                                int flags = wxEXEC_ASYNC,
-WXDLLIMPEXP_BASE long wxExecute(const wxString& command, int flags = wxEXEC_ASYNC,
+                                wxProcess *process = NULL);
-                           wxProcess *process = (wxProcess *) NULL);
+WXDLLIMPEXP_BASE long wxExecute(char **argv,
+                                int flags = wxEXEC_ASYNC,
+                                wxProcess *process = NULL);
+WXDLLIMPEXP_BASE long wxExecute(const wxString& command,
+                                int flags = wxEXEC_ASYNC,
+                                wxProcess *process = NULL);
 
 // execute the command capturing its output into an array line by line, this is
 // always synchronous

interface/utils.h

@@ -624,8 +624,10 @@ wxString wxGetUserHome(const wxString& user = "");
         array, any additional ones are the command parameters and the array must be
         terminated with a @NULL pointer.
     @param flags
-        Combination of bit masks wxEXEC_ASYNC,
+        Must include either wxEXEC_ASYNC or wxEXEC_SYNC and can also include
-        wxEXEC_SYNC and wxEXEC_NOHIDE
+        wxEXEC_NOHIDE, wxEXEC_MAKE_GROUP_LEADER (in either case) or
+        wxEXEC_NODISABLE and wxEXEC_NOEVENTS or wxEXEC_BLOCK, which is equal to
+        their combination, in wxEXEC_SYNC case.
     @param callback
         An optional pointer to wxProcess
 
@@ -633,15 +635,19 @@ wxString wxGetUserHome(const wxString& user = "");
 
     @header{wx/utils.h}
 */
-long wxExecute(const wxString& command, int sync = wxEXEC_ASYNC,
+long wxExecute(const wxString& command,
+               int sync = wxEXEC_ASYNC,
                wxProcess* callback = NULL);
-wxPerl note: long wxExecute(char** argv,
+long wxExecute(char** argv,
                int flags = wxEXEC_ASYNC,
                wxProcess* callback = NULL);
-wxPerl note: long wxExecute(const wxString& command,
+long wxExecute(wchar_t** argv,
+               int flags = wxEXEC_ASYNC,
+               wxProcess* callback = NULL);
+long wxExecute(const wxString& command,
                wxArrayString& output,
                int flags = 0);
-wxPerl note: long wxExecute(const wxString& command,
+long wxExecute(const wxString& command,
                wxArrayString& output,
                wxArrayString& errors,
                int flags = 0);

src/mac/corefoundation/utilsexc_base.cpp

@@ -1,6 +1,6 @@
 /////////////////////////////////////////////////////////////////////////////
 // Name:        mac/corefoundation/utilsexc_base.cpp
-// Purpose:     wxMacExecute
+// Purpose:     wxMacLaunch
 // Author:      Ryan Norton
 // Modified by:
 // Created:     2005-06-21
@@ -48,25 +48,19 @@
 
 //+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 //
-//    wxMacExecute
+//    wxMacLaunch
 //
 // argv is the command line split up, with the application path first
 // flags are the flags from wxExecute
 // process is the process passed from wxExecute for pipe streams etc.
 // returns -1 on error for wxEXEC_SYNC and 0 on error for wxEXEC_ASYNC
 //+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-long wxMacExecute(wxChar **argv,
+bool wxMacLaunch(char **argv)
-               int flags,
-               wxProcess *WXUNUSED(process))
 {
-    // Semi-macros used for return value of wxMacExecute
-    const long errorCode = ((flags & wxEXEC_SYNC) ? -1 : 0);
-    const long successCode = ((flags & wxEXEC_SYNC) ? 0 : -1); // fake PID
-
     // Obtains the number of arguments for determining the size of
     // the CFArray used to hold them
     CFIndex cfiCount = 0;
-    for(wxChar** argvcopy = argv; *argvcopy != NULL ; ++argvcopy)
+    for(char** argvcopy = argv; *argvcopy != NULL ; ++argvcopy)
     {
         ++cfiCount;
     }
@@ -75,8 +69,8 @@ long wxMacExecute(wxChar **argv,
     // to launch
     if(cfiCount == 0)
     {
-        wxLogDebug(wxT("wxMacExecute No file to launch!"));
+        wxLogDebug(wxT("wxMacLaunch No file to launch!"));
-        return errorCode ;
+        return false ;
     }
 
     // Path to bundle
@@ -94,8 +88,8 @@ long wxMacExecute(wxChar **argv,
     // Check for error from the CFURL
     if(!cfurlApp)
     {
-        wxLogDebug(wxT("wxMacExecute Can't open path: %s"), path.c_str());
+        wxLogDebug(wxT("wxMacLaunch Can't open path: %s"), path.c_str());
-        return errorCode ;
+        return false ;
     }
 
     // Create a CFBundle from the CFURL created earlier
@@ -106,9 +100,9 @@ long wxMacExecute(wxChar **argv,
     // at all (maybe a simple directory etc.)
     if(!cfbApp)
     {
-        wxLogDebug(wxT("wxMacExecute Bad bundle: %s"), path.c_str());
+        wxLogDebug(wxT("wxMacLaunch Bad bundle: %s"), path.c_str());
         CFRelease(cfurlApp);
-        return errorCode ;
+        return false ;
     }
 
     // Get the bundle type and make sure its an 'APPL' bundle
@@ -117,10 +111,10 @@ long wxMacExecute(wxChar **argv,
     CFBundleGetPackageInfo(cfbApp, &dwBundleType, &dwBundleCreator);
     if(dwBundleType != 'APPL')
     {
-        wxLogDebug(wxT("wxMacExecute Not an APPL bundle: %s"), path.c_str());
+        wxLogDebug(wxT("wxMacLaunch Not an APPL bundle: %s"), path.c_str());
         CFRelease(cfbApp);
         CFRelease(cfurlApp);
-        return errorCode ;
+        return false ;
     }
 
     // Create a CFArray for dealing with the command line
@@ -129,10 +123,10 @@ long wxMacExecute(wxChar **argv,
                                     cfiCount-1, &kCFTypeArrayCallBacks);
     if(!cfaFiles) //This should never happen
     {
-        wxLogDebug(wxT("wxMacExecute Could not create CFMutableArray"));
+        wxLogDebug(wxT("wxMacLaunch Could not create CFMutableArray"));
         CFRelease(cfbApp);
         CFRelease(cfurlApp);
-        return errorCode ;
+        return false ;
     }
 
     // Loop through command line arguments to the bundle,
@@ -184,7 +178,7 @@ long wxMacExecute(wxChar **argv,
         if(!cfurlCurrentFile)
         {
             wxLogDebug(
-                wxT("wxMacExecute Could not create CFURL for argument:%s"),
+                wxT("wxMacLaunch Could not create CFURL for argument:%s"),
                 *argv);
             continue;
         }
@@ -222,12 +216,12 @@ long wxMacExecute(wxChar **argv,
     // Check for error from LSOpenFromURLSpec
     if(status != noErr)
     {
-        wxLogDebug(wxT("wxMacExecute LSOpenFromURLSpec Error: %d"),
+        wxLogDebug(wxT("wxMacLaunch LSOpenFromURLSpec Error: %d"),
                    (int)status);
-        return errorCode ;
+        return false ;
     }
 
     // No error from LSOpenFromURLSpec, so app was launched
-    return successCode;
+    return true ;
 }
 

src/msw/utilsexc.cpp

@@ -1012,9 +1012,11 @@ long wxExecute(const wxString& cmd, int flags, wxProcess *handler)
     return dwExitCode;
 }
 
-long wxExecute(wxChar **argv, int flags, wxProcess *handler)
+template <typename CharType>
+long wxExecuteImpl(CharType **argv, int flags, wxProcess *handler)
 {
     wxString command;
+    command.reserve(1024);
 
     for ( ;; )
     {
@@ -1022,8 +1024,18 @@ long wxExecute(wxChar **argv, int flags, wxProcess *handler)
         if ( !*argv )
             break;
 
-        command += _T(' ');
+        command += ' ';
     }
 
     return wxExecute(command, flags, handler);
 }
+
+long wxExecute(char **argv, int flags, wxProcess *handler)
+{
+    return wxExecuteImpl(argv, flags, handler);
+}
+
+long wxExecute(wchar_t **argv, int flags, wxProcess *handler)
+{
+    return wxExecuteImpl(argv, flags, handler);
+}

src/unix/utilsunx.cpp

@@ -263,139 +263,6 @@ int wxKill(long pid, wxSignal sig, wxKillError *rc, int flags)
     return err;
 }
 
-#define WXEXECUTE_NARGS   127
-
-#if defined(__DARWIN__)
-long wxMacExecute(wxChar **argv,
-               int flags,
-               wxProcess *process);
-#endif
-
-long wxExecute( const wxString& command, int flags, wxProcess *process )
-{
-    wxCHECK_MSG( !command.empty(), 0, wxT("can't exec empty command") );
-
-    wxLogTrace(wxT("exec"), wxT("Executing \"%s\""), command.c_str());
-
-#if wxUSE_THREADS
-    // fork() doesn't mix well with POSIX threads: on many systems the program
-    // deadlocks or crashes for some reason. Probably our code is buggy and
-    // doesn't do something which must be done to allow this to work, but I
-    // don't know what yet, so for now just warn the user (this is the least we
-    // can do) about it
-    wxASSERT_MSG( wxThread::IsMain(),
-                    _T("wxExecute() can be called only from the main thread") );
-#endif // wxUSE_THREADS
-
-    int argc = 0;
-    wxChar *argv[WXEXECUTE_NARGS];
-    wxString argument;
-    const wxChar *cptr = command.c_str();
-    wxChar quotechar = wxT('\0'); // is arg quoted?
-    bool escaped = false;
-
-    // split the command line in arguments
-    do
-    {
-        argument = wxEmptyString;
-        quotechar = wxT('\0');
-
-        // eat leading whitespace:
-        while ( wxIsspace(*cptr) )
-            cptr++;
-
-        if ( *cptr == wxT('\'') || *cptr == wxT('"') )
-            quotechar = *cptr++;
-
-        do
-        {
-            if ( *cptr == wxT('\\') && ! escaped )
-            {
-                escaped = true;
-                cptr++;
-                continue;
-            }
-
-            // all other characters:
-            argument += *cptr++;
-            escaped = false;
-
-            // have we reached the end of the argument?
-            if ( (*cptr == quotechar && ! escaped)
-                 || (quotechar == wxT('\0') && wxIsspace(*cptr))
-                 || *cptr == wxT('\0') )
-            {
-                wxASSERT_MSG( argc < WXEXECUTE_NARGS,
-                              wxT("too many arguments in wxExecute") );
-
-                argv[argc] = new wxChar[argument.length() + 1];
-                wxStrcpy(argv[argc], argument.c_str());
-                argc++;
-
-                // if not at end of buffer, swallow last character:
-                if(*cptr)
-                    cptr++;
-
-                break; // done with this one, start over
-            }
-        } while(*cptr);
-    } while(*cptr);
-    argv[argc] = NULL;
-
-    long lRc;
-#if defined(__DARWIN__)
-    // wxMacExecute only executes app bundles.
-    // It returns an error code if the target is not an app bundle, thus falling
-    // through to the regular wxExecute for non app bundles.
-    lRc = wxMacExecute(argv, flags, process);
-    if( lRc != ((flags & wxEXEC_SYNC) ? -1 : 0))
-        return lRc;
-#endif
-
-    // do execute the command
-    lRc = wxExecute(argv, flags, process);
-
-    // clean up
-    argc = 0;
-    while( argv[argc] )
-        delete [] argv[argc++];
-
-    return lRc;
-}
-
-// ----------------------------------------------------------------------------
-// wxShell
-// ----------------------------------------------------------------------------
-
-static wxString wxMakeShellCommand(const wxString& command)
-{
-    wxString cmd;
-    if ( !command )
-    {
-        // just an interactive shell
-        cmd = _T("xterm");
-    }
-    else
-    {
-        // execute command in a shell
-        cmd << _T("/bin/sh -c '") << command << _T('\'');
-    }
-
-    return cmd;
-}
-
-bool wxShell(const wxString& command)
-{
-    return wxExecute(wxMakeShellCommand(command), wxEXEC_SYNC) == 0;
-}
-
-bool wxShell(const wxString& command, wxArrayString& output)
-{
-    wxCHECK_MSG( !command.empty(), false, _T("can't exec shell non interactively") );
-
-    return wxExecute(wxMakeShellCommand(command), output);
-}
-
 // Shutdown or reboot the PC
 bool wxShutdown(wxShutdownFlags wFlags)
 {
@@ -465,10 +332,174 @@ bool wxPipeInputStream::CanRead() const
 #endif // HAS_PIPE_INPUT_STREAM
 
 // ----------------------------------------------------------------------------
-// wxExecute: the real worker function
+// wxShell
 // ----------------------------------------------------------------------------
 
-long wxExecute(wxChar **argv, int flags, wxProcess *process)
+static wxString wxMakeShellCommand(const wxString& command)
+{
+    wxString cmd;
+    if ( !command )
+    {
+        // just an interactive shell
+        cmd = _T("xterm");
+    }
+    else
+    {
+        // execute command in a shell
+        cmd << _T("/bin/sh -c '") << command << _T('\'');
+    }
+
+    return cmd;
+}
+
+bool wxShell(const wxString& command)
+{
+    return wxExecute(wxMakeShellCommand(command), wxEXEC_SYNC) == 0;
+}
+
+bool wxShell(const wxString& command, wxArrayString& output)
+{
+    wxCHECK_MSG( !command.empty(), false, _T("can't exec shell non interactively") );
+
+    return wxExecute(wxMakeShellCommand(command), output);
+}
+
+namespace
+{
+
+// helper class for storing arguments as char** array suitable for passing to
+// execvp(), whatever form they were passed to us
+class ArgsArray
+{
+public:
+    ArgsArray(const wxArrayString& args)
+    {
+        Init(args.size());
+
+        for ( int i = 0; i < m_argc; i++ )
+        {
+            m_argv[i] = wxStrdup(args[i]);
+        }
+    }
+
+    ArgsArray(wchar_t **wargv)
+    {
+        int argc = 0;
+        while ( *wargv++ )
+            argc++;
+
+        Init(argc);
+
+        for ( int i = 0; i < m_argc; i++ )
+        {
+            m_argv[i] = wxSafeConvertWX2MB(wargv[i]).release();
+        }
+    }
+
+    ~ArgsArray()
+    {
+        for ( int i = 0; i < m_argc; i++ )
+        {
+            free(m_argv[i]);
+        }
+
+        delete [] m_argv;
+    }
+
+    operator char**() const { return m_argv; }
+
+private:
+    void Init(int argc)
+    {
+        m_argc = argc;
+        m_argv = new char *[m_argc + 1];
+        m_argv[m_argc] = NULL;
+    }
+
+    int m_argc;
+    char **m_argv;
+
+    DECLARE_NO_COPY_CLASS(ArgsArray);
+};
+
+} // anonymous namespace
+
+// ----------------------------------------------------------------------------
+// wxExecute implementations
+// ----------------------------------------------------------------------------
+
+#if defined(__DARWIN__)
+bool wxMacLaunch(char **argv);
+#endif
+
+long wxExecute(const wxString& command, int flags, wxProcess *process)
+{
+    wxArrayString args;
+
+    const char *cptr = command.c_str();
+
+    // split the command line in arguments
+    //
+    // TODO: combine this with wxCmdLineParser::ConvertStringToArgs(), it
+    //       doesn't do exactly the same thing right now but it's pretty close
+    //       and we shouldn't maintain 2 copies of this code
+    do
+    {
+        wxString argument;
+        char quotechar = '\0'; // is arg quoted?
+        bool escaped = false;
+
+        // eat leading whitespace:
+        while ( wxIsspace(*cptr) )
+            cptr++;
+
+        if ( *cptr == '\'' || *cptr == '"' )
+            quotechar = *cptr++;
+
+        do
+        {
+            if ( *cptr == '\\' && !escaped )
+            {
+                escaped = true;
+                cptr++;
+                continue;
+            }
+
+            // all other characters:
+            argument += *cptr++;
+            escaped = false;
+
+            // have we reached the end of the argument?
+            if ( (*cptr == quotechar && !escaped)
+                 || (quotechar == '\0' && wxIsspace(*cptr))
+                 || *cptr == '\0' )
+            {
+                args.push_back(argument);
+
+                // if not at end of buffer, swallow last character:
+                if ( *cptr )
+                    cptr++;
+
+                break; // done with this one, start over
+            }
+        } while ( *cptr );
+    } while ( *cptr );
+
+    ArgsArray argv(args);
+
+    // do execute the command
+    return wxExecute(argv, flags, process);
+}
+
+long wxExecute(wchar_t **wargv, int flags, wxProcess *process)
+{
+    ArgsArray argv(wargv);
+
+    return wxExecute(argv, flags, process);
+}
+
+// wxExecute: the real worker function
+long wxExecute(char **argv, int flags, wxProcess *process)
 {
     // for the sync execution, we return -1 to indicate failure, but for async
     // case we return 0 which is never a valid PID
@@ -479,38 +510,29 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
 
     wxCHECK_MSG( *argv, ERROR_RETURN_CODE, wxT("can't exec empty command") );
 
-#if wxUSE_UNICODE
+#if wxUSE_THREADS
-    int mb_argc = 0;
+    // fork() doesn't mix well with POSIX threads: on many systems the program
-    char *mb_argv[WXEXECUTE_NARGS];
+    // deadlocks or crashes for some reason. Probably our code is buggy and
+    // doesn't do something which must be done to allow this to work, but I
+    // don't know what yet, so for now just warn the user (this is the least we
+    // can do) about it
+    wxASSERT_MSG( wxThread::IsMain(),
+                    _T("wxExecute() can be called only from the main thread") );
+#endif // wxUSE_THREADS
 
-    while (argv[mb_argc])
+#if defined(__DARWIN__)
+    // wxMacLaunch() only executes app bundles and only does it asynchronously.
+    // It returns false if the target is not an app bundle, thus falling
+    // through to the regular code for non app bundles.
+    if ( !(flags & wxEXEC_SYNC) && wxMacLaunch(argv) )
     {
-        wxWX2MBbuf mb_arg = wxSafeConvertWX2MB(argv[mb_argc]);
+        // we don't have any PID to return so just make up something non null
-        mb_argv[mb_argc] = strdup(mb_arg);
+        return -1;
-        mb_argc++;
     }
-    mb_argv[mb_argc] = (char *) NULL;
+#endif // __DARWIN__
 
-    // this macro will free memory we used above
-    #define ARGS_CLEANUP                                 \
-        for ( mb_argc = 0; mb_argv[mb_argc]; mb_argc++ ) \
-            free(mb_argv[mb_argc])
-#else // ANSI
-    // no need for cleanup
-    #define ARGS_CLEANUP
 
-    wxChar **mb_argv = argv;
+    // this struct contains all information which we use for housekeeping
-#endif // Unicode/ANSI
-
-    // we want this function to work even if there is no wxApp so ensure that
-    // we have a valid traits pointer
-    wxConsoleAppTraits traitsConsole;
-    wxAppTraits *traits = wxTheApp ? wxTheApp->GetTraits() : NULL;
-    if ( !traits )
-        traits = &traitsConsole;
-
-    // this struct contains all information which we pass to and from
-    // wxAppTraits methods
     wxExecuteData execData;
     execData.flags = flags;
     execData.process = process;
@@ -520,8 +542,6 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
     {
         wxLogError( _("Failed to execute '%s'\n"), *argv );
 
-        ARGS_CLEANUP;
-
         return ERROR_RETURN_CODE;
     }
 
@@ -536,8 +556,6 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
         {
             wxLogError( _("Failed to execute '%s'\n"), *argv );
 
-            ARGS_CLEANUP;
-
             return ERROR_RETURN_CODE;
         }
     }
@@ -557,8 +575,6 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
     {
         wxLogSysError( _("Fork failed") );
 
-        ARGS_CLEANUP;
-
         return ERROR_RETURN_CODE;
     }
     else if ( pid == 0 )  // we're in child
@@ -618,12 +634,11 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
             pipeErr.Close();
         }
 
-        execvp (*mb_argv, mb_argv);
+        execvp(*argv, argv);
 
         fprintf(stderr, "execvp(");
-        // CS changed ppc to ppc_ as ppc is not available under mac os CW Mach-O
+        for ( char **a = argv; *a; a++ )
-        for ( char **ppc_ = mb_argv; *ppc_; ppc_++ )
+            fprintf(stderr, "%s%s", a == argv ? "" : ", ", *a);
-            fprintf(stderr, "%s%s", ppc_ == mb_argv ? "" : ", ", *ppc_);
         fprintf(stderr, ") failed with error %d!\n", errno);
 
         // there is no return after successful exec()
@@ -641,8 +656,6 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
     }
     else // we're in parent
     {
-        ARGS_CLEANUP;
-
         // save it for WaitForChild() use
         execData.pid = pid;
 
@@ -685,6 +698,13 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
             pipeErr.Close();
         }
 
+        // we want this function to work even if there is no wxApp so ensure
+        // that we have a valid traits pointer
+        wxConsoleAppTraits traitsConsole;
+        wxAppTraits *traits = wxTheApp ? wxTheApp->GetTraits() : NULL;
+        if ( !traits )
+            traits = &traitsConsole;
+
         return traits->WaitForChild(execData);
     }
 
@@ -694,7 +714,6 @@ long wxExecute(wxChar **argv, int flags, wxProcess *process)
 }
 
 #undef ERROR_RETURN_CODE
-#undef ARGS_CLEANUP
 
 // ----------------------------------------------------------------------------
 // file and directory functions