Changeset 63153 in vbox for trunk/src/VBox/Main

Timestamp:

Aug 8, 2016 12:00:20 PM (9 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

109771

Message:

Split up GuestProcess:i_startProcess to deal with uninitialized variables and an misplaced AssertReturn (didn't clean up the event).

Location:

trunk/src/VBox/Main

Files:

• include/GuestProcessImpl.h (modified) (1 diff)
• src-client/GuestProcessImpl.cpp (modified) (8 diffs)

trunk/src/VBox/Main/include/GuestProcessImpl.h

@@ -53 +53 @@
     int i_onRemove(void);
     int i_readData(uint32_t uHandle, uint32_t uSize, uint32_t uTimeoutMS, void *pvData, size_t cbData, uint32_t *pcbRead, int *pGuestRc);
-    int i_startProcess(uint32_t uTimeoutMS, int *pGuestRc);
+    int i_startProcess(uint32_t cMsTimeout, int *pGuestRc);
+    int i_startProcessInner(uint32_t cMsTimeout, AutoWriteLock &rLock, GuestWaitEvent *pEvent, int *pGuestRc);
     int i_startProcessAsync(void);
     int i_terminateProcess(uint32_t uTimeoutMS, int *pGuestRc);

trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp

@@ -1006 +1006 @@
 }
 
-int GuestProcess::i_startProcess(uint32_t uTimeoutMS, int *pGuestRc)
-{
-    LogFlowThisFunc(("uTimeoutMS=%RU32, procExe=%s, procTimeoutMS=%RU32, procFlags=%x, sessionID=%RU32\n",
-                     uTimeoutMS, mData.mProcess.mExecutable.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags,
+int GuestProcess::i_startProcess(uint32_t cMsTimeout, int *pGuestRc)
+{
+    LogFlowThisFunc(("cMsTimeout=%RU32, procExe=%s, procTimeoutMS=%RU32, procFlags=%x, sessionID=%RU32\n",
+                     cMsTimeout, mData.mProcess.mExecutable.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags,
                      mSession->i_getId()));
 
@@ -1025 +1025 @@
     {
         eventTypes.push_back(VBoxEventType_OnGuestProcessStateChanged);
-
         vrc = registerWaitEvent(eventTypes, &pEvent);
     }
@@ -1032 +1031 @@
         vrc = VERR_NO_MEMORY;
     }
-
     if (RT_FAILURE(vrc))
         return vrc;
 
+    vrc = i_startProcessInner(cMsTimeout, alock, pEvent, pGuestRc);
+
+    unregisterWaitEvent(pEvent);
+
+    LogFlowFuncLeaveRC(vrc);
+    return vrc;
+}
+
+int GuestProcess::i_startProcessInner(uint32_t cMsTimeout, AutoWriteLock &rLock, GuestWaitEvent *pEvent, int *pGuestRc)
+{
     GuestSession *pSession = mSession;
     AssertPtr(pSession);
@@ -1044 +1052 @@
 
     /* Prepare arguments. */
+    size_t cArgs = mData.mProcess.mArguments.size();
+    if (cArgs >= 128*1024)
+        return VERR_BUFFER_OVERFLOW;
+
     char *pszArgs = NULL;
-    size_t cArgs = mData.mProcess.mArguments.size();
-    if (cArgs >= UINT32_MAX)
-        vrc = VERR_BUFFER_OVERFLOW;
-
-    if (   RT_SUCCESS(vrc)
-        && cArgs)
+    int vrc = VINF_SUCCESS;
+    if (cArgs)
     {
         char const **papszArgv = (char const **)RTMemAlloc((cArgs + 1) * sizeof(papszArgv[0]));
@@ -1068 +1076 @@
 
         RTMemFree(papszArgv);
+        if (RT_FAILURE(vrc))
+            return vrc;
+
+        /* Note! No returns after this. */
     }
 
     /* Calculate arguments size (in bytes). */
-    size_t cbArgs = 0;
-    if (RT_SUCCESS(vrc))
-        cbArgs = pszArgs ? strlen(pszArgs) + 1 : 0; /* Include terminating zero. */
+    size_t cbArgs = pszArgs ? strlen(pszArgs) + 1 : 0; /* Include terminating zero. */
 
     /* Prepare environment.  The guest service dislikes the empty string at the end, so drop it. */
     size_t  cbEnvBlock;
     char   *pszzEnvBlock;
-    if (RT_SUCCESS(vrc))
-        vrc = mData.mProcess.mEnvironmentChanges.queryUtf8Block(&pszzEnvBlock, &cbEnvBlock);
+    vrc = mData.mProcess.mEnvironmentChanges.queryUtf8Block(&pszzEnvBlock, &cbEnvBlock);
     if (RT_SUCCESS(vrc))
     {
@@ -1124 +1133 @@
         }
 
-        alock.release(); /* Drop the write lock before sending. */
+        rLock.release(); /* Drop the write lock before sending. */
 
         vrc = sendCommand(HOST_EXEC_CMD, i, paParms);
@@ -1136 +1145 @@
     }
 
-    if (pszArgs)
-        RTStrFree(pszArgs);
+    RTStrFree(pszArgs);
 
     if (RT_SUCCESS(vrc))
-        vrc = i_waitForStatusChange(pEvent, uTimeoutMS,
+        vrc = i_waitForStatusChange(pEvent, cMsTimeout,
                                     NULL /* Process status */, pGuestRc);
-    unregisterWaitEvent(pEvent);
-
-    LogFlowFuncLeaveRC(vrc);
     return vrc;
 }
@@ -1327 +1332 @@
         case ProcessStatus_Undefined:
         case ProcessStatus_Starting:
+        case ProcessStatus_Terminating:
+        case ProcessStatus_Paused:
             /* No result available yet, leave wait
              * flags untouched. */