Index: /trunk/src/VBox/HostServices/GuestControl/service.cpp =================================================================== --- /trunk/src/VBox/HostServices/GuestControl/service.cpp (revision 29866) +++ /trunk/src/VBox/HostServices/GuestControl/service.cpp (revision 29867) @@ -28,25 +28,25 @@ * new host commands to perform. There can be multiple clients connected * to a service. A client is represented by its HGCM client ID. - * - Context ID: A (almost) unique ID automatically generated on the host (Main API) + * - Context ID: A (almost) unique ID automatically generated on the host (Main API) * to not only distinguish clients but individual requests. Because * the host does not know anything about connected clients it needs * an indicator which it can refer to later. This context ID gets - * internally bound by the service to a client which actually processes + * internally bound by the service to a client which actually processes * the command in order to have a relationship between client<->context ID(s). * * The host can trigger commands which get buffered by the service (with full HGCM - * parameter info). As soon as a client connects (or is ready to do some new work) - * it gets a buffered host command to process it. This command then will be immediately + * parameter info). As soon as a client connects (or is ready to do some new work) + * it gets a buffered host command to process it. This command then will be immediately * removed from the command list. If there are ready clients but no new commands to be * processed, these clients will be set into a deferred state (that is being blocked * to return until a new command is available). * - * If a client needs to inform the host that something happend, it can send a + * If a client needs to inform the host that something happend, it can send a * message to a low level HGCM callback registered in Main. This callback contains - * the actual data as well as the context ID to let the host do the next necessary + * the actual data as well as the context ID to let the host do the next necessary * steps for this context. This context ID makes it possible to wait for an event * inside the host's Main API function (like starting a process on the guest and * wait for getting its PID returned by the client) as well as cancelling blocking - * host calls in order the client terminated/crashed (HGCM detects disconnected + * host calls in order the client terminated/crashed (HGCM detects disconnected * clients and reports it to this service's callback). */ @@ -80,5 +80,5 @@ /** * Structure for holding all clients with their - * generated host contexts. This is necessary for + * generated host contexts. This is necessary for * mainting the relationship between a client and its context IDs. */ @@ -135,5 +135,5 @@ GuestCall() : mClientID(0), mHandle(0), mParms(NULL), mNumParms(0) {} /** The normal contructor. */ - GuestCall(uint32_t aClientID, VBOXHGCMCALLHANDLE aHandle, + GuestCall(uint32_t aClientID, VBOXHGCMCALLHANDLE aHandle, VBOXHGCMSVCPARM aParms[], uint32_t cParms) : mClientID(aClientID), mHandle(aHandle), mParms(aParms), mNumParms(cParms) {} @@ -475,5 +475,5 @@ AssertPtr(pCmd); int rc; - + /* Sufficient parameter space? */ if (pCmd->mParmBuf.uParmCount > cParms) @@ -481,5 +481,5 @@ paParms[0].setUInt32(pCmd->mParmBuf.uMsg); /* Message ID */ paParms[1].setUInt32(pCmd->mParmBuf.uParmCount); /* Required parameters for message */ - + /* * So this call apparently failed because the guest wanted to peek @@ -500,10 +500,10 @@ * defer the guest call until we have something from the host. */ -int Service::retrieveNextHostCmd(uint32_t u32ClientID, VBOXHGCMCALLHANDLE callHandle, +int Service::retrieveNextHostCmd(uint32_t u32ClientID, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]) { int rc = VINF_SUCCESS; - /* + /* * Lookup client in our list so that we can assign the context ID of * a command to that client. @@ -547,4 +547,5 @@ * later reference & cleanup). */ Assert(curCmd.mContextID > 0); + /// @todo r=bird: check if already in the list. it->mContextList.push_back(curCmd.mContextID); @@ -568,12 +569,12 @@ { if (it->mClientID == u32ClientID) - { + { if (it->mNumParms >= 2) { it->mParms[0].setUInt32(GETHOSTMSG_EXEC_HOST_CANCEL_WAIT); /* Message ID. */ it->mParms[1].setUInt32(0); /* Required parameters for message. */ - } + } if (mpHelpers) - mpHelpers->pfnCallComplete(it->mHandle, rc); + mpHelpers->pfnCallComplete(it->mHandle, rc); it = mClientList.erase(it); } @@ -636,5 +637,5 @@ && newCmd.mParmBuf.uParmCount > 0) { - /* + /* * Assume that the context ID *always* is the first parameter, * assign the context ID to the command. @@ -651,5 +652,5 @@ { GuestCall guest = mClientList.front(); - rc = sendHostCmdToGuest(&newCmd, + rc = sendHostCmdToGuest(&newCmd, guest.mHandle, guest.mNumParms, guest.mParms); @@ -657,8 +658,8 @@ AssertPtr(mpHelpers); mpHelpers->pfnCallComplete(guest.mHandle, rc); - mClientList.pop_front(); - + mClientList.pop_front(); + /* If we got VERR_TOO_MUCH_DATA we buffer the host command in the next block - * and return success to the host. */ + * and return success to the host. */ if (rc == VERR_TOO_MUCH_DATA) { @@ -675,5 +676,5 @@ if (!fProcessed) { - mHostCmds.push_back(newCmd); + mHostCmds.push_back(newCmd); #if 0 /* Limit list size by deleting oldest element. */ Index: /trunk/src/VBox/Main/GuestImpl.cpp =================================================================== --- /trunk/src/VBox/Main/GuestImpl.cpp (revision 29866) +++ /trunk/src/VBox/Main/GuestImpl.cpp (revision 29867) @@ -599,9 +599,9 @@ /* Not found, add to list. */ GuestProcess p; - p.mPID = pCBData->u32PID; + p.mPID = pCBData->u32PID; p.mStatus = pCBData->u32Status; p.mExitCode = pCBData->u32Flags; /* Contains exit code. */ p.mFlags = 0; - + mGuestProcessList.push_back(p); } @@ -747,7 +747,7 @@ if (SUCCEEDED(it->pProgress->COMGETTER(Canceled)(&fCancelled)) && !fCancelled) it->pProgress->Cancel(); - /* - * Do *not NULL pProgress here, because waiting function like executeProcess() - * will still rely on this object for checking whether they have to give up! + /* + * Do *not NULL pProgress here, because waiting function like executeProcess() + * will still rely on this object for checking whether they have to give up! */ } @@ -777,7 +777,8 @@ { AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + /// @todo r=bird: check if already in the list and find another one. mCallbackList.push_back(context); nCallbacks = mCallbackList.size(); - } + } #if 0 @@ -803,4 +804,6 @@ ULONG aTimeoutMS, ULONG *aPID, IProgress **aProgress) { +/** @todo r=bird: Eventually we should clean up all the timeout parameters + * in the API and have the same way of specifying infinite waits! */ #ifndef VBOX_WITH_GUEST_CONTROL ReturnComNotImplemented(); @@ -860,4 +863,5 @@ for (unsigned i = 0; RT_SUCCESS(vrc) && i < uNumArgs; i++) { + /// @todo r=bird: RTUtf16ToUtf8(). int cbLen = RTStrAPrintf(&papszArgv[i], "%s", Utf8Str(args[i]).raw()); if (cbLen < 0) @@ -901,5 +905,5 @@ PCALLBACKDATAEXECSTATUS pData = (PCALLBACKDATAEXECSTATUS)RTMemAlloc(sizeof(CALLBACKDATAEXECSTATUS)); AssertReturn(pData, VBOX_E_IPRT_ERROR); - uContextID = addCtrlCallbackContext(VBOXGUESTCTRLCALLBACKTYPE_EXEC_START, + uContextID = addCtrlCallbackContext(VBOXGUESTCTRLCALLBACKTYPE_EXEC_START, pData, sizeof(CALLBACKDATAEXECSTATUS), progress); Assert(uContextID > 0); @@ -921,9 +925,9 @@ VMMDev *vmmDev; { - /* Make sure mParent is valid, so set the read lock while using. + /* Make sure mParent is valid, so set the read lock while using. * Do not keep this lock while doing the actual call, because in the meanwhile * another thread could request a write lock which would be a bad idea ... */ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + /* Forward the information to the VMM device. */ AssertPtr(mParent); @@ -973,10 +977,13 @@ /* Check for manual stop. */ if (!it->pProgress.isNull()) - { + { rc = it->pProgress->COMGETTER(Canceled)(&fCanceled); if (FAILED(rc)) throw rc; if (fCanceled) - break; /* Client wants to abort. */ + break; /* HGCM/guest wants to abort because of status change. */ + } + /// @todo r=bird: two operation progress object and wait first operation. + /// IProgress::WaitForOperationCompletion. RTThreadSleep(cMsWait); } @@ -986,5 +993,5 @@ if (!fCanceled) { - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); + AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); PCALLBACKDATAEXECSTATUS pData = (PCALLBACKDATAEXECSTATUS)it->pvData; @@ -1001,5 +1008,5 @@ *aPID = pData->u32PID; break; - + /* In any other case the process either already * terminated or something else went wrong, so no PID ... */ @@ -1010,15 +1017,15 @@ case PROC_STS_TOA: case PROC_STS_DWN: - /* + /* * Process (already) ended, but we want to get the - * PID anyway to retrieve the output in a later call. + * PID anyway to retrieve the output in a later call. */ *aPID = pData->u32PID; break; - + case PROC_STS_ERROR: vrc = pData->u32Flags; /* u32Flags member contains IPRT error code. */ break; - + default: vrc = VERR_INVALID_PARAMETER; /* Unknown status, should never happen! */ @@ -1073,5 +1080,5 @@ rc = setError(E_UNEXPECTED, tr("The service call failed with error %Rrc"), vrc); - } + } } else /* Execution went fine. */ @@ -1121,4 +1128,6 @@ STDMETHODIMP Guest::GetProcessOutput(ULONG aPID, ULONG aFlags, ULONG aTimeoutMS, ULONG64 aSize, ComSafeArrayOut(BYTE, aData)) { +/** @todo r=bird: Eventually we should clean up all the timeout parameters + * in the API and have the same way of specifying infinite waits! */ #ifndef VBOX_WITH_GUEST_CONTROL ReturnComNotImplemented(); @@ -1132,5 +1141,5 @@ AutoCaller autoCaller(this); - if (FAILED(autoCaller.rc())) return autoCaller.rc(); + if (FAILED(autoCaller.rc())) return autoCaller.rc(); HRESULT rc = S_OK; @@ -1163,8 +1172,8 @@ pData, sizeof(CALLBACKDATAEXECOUT), progress); Assert(uContextID > 0); - + size_t cbData = (size_t)RT_MIN(aSize, _64K); com::SafeArray outputData(cbData); - + VBOXHGCMSVCPARM paParms[5]; int i = 0; @@ -1172,15 +1181,15 @@ paParms[i++].setUInt32(aPID); paParms[i++].setUInt32(aFlags); /** @todo Should represent stdout and/or stderr. */ - + int vrc = VINF_SUCCESS; - + { VMMDev *vmmDev; { - /* Make sure mParent is valid, so set the read lock while using. + /* Make sure mParent is valid, so set the read lock while using. * Do not keep this lock while doing the actual call, because in the meanwhile * another thread could request a write lock which would be a bad idea ... */ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + /* Forward the information to the VMM device. */ AssertPtr(mParent); @@ -1195,9 +1204,9 @@ } } - + if (RT_SUCCESS(vrc)) { LogFlowFunc(("Waiting for HGCM callback (timeout=%ldms) ...\n", aTimeoutMS)); - + /* * Wait for the HGCM low level callback until the process @@ -1233,6 +1242,6 @@ } RTThreadSleep(cMsWait); - } - + } + /* Was the whole thing canceled? */ if (!fCanceled) @@ -1241,10 +1250,10 @@ { AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + /* Did we get some output? */ pData = (PCALLBACKDATAEXECOUT)it->pvData; Assert(it->cbData == sizeof(CALLBACKDATAEXECOUT)); AssertPtr(pData); - + if (pData->cbData) { @@ -1252,5 +1261,5 @@ if (pData->cbData > cbData) outputData.resize(pData->cbData); - + /* Fill output in supplied out buffer. */ memcpy(outputData.raw(), pData->pvData, pData->cbData); @@ -1296,10 +1305,10 @@ else /* PID lookup failed. */ rc = setError(VBOX_E_IPRT_ERROR, - tr("Process (PID %u) not found!"), aPID); + tr("Process (PID %u) not found!"), aPID); } else /* HGCM operation failed. */ rc = setError(E_UNEXPECTED, tr("The HGCM call failed with error %Rrc"), vrc); - + /* Cleanup. */ progress->uninit(); @@ -1328,5 +1337,5 @@ AutoCaller autoCaller(this); - if (FAILED(autoCaller.rc())) return autoCaller.rc(); + if (FAILED(autoCaller.rc())) return autoCaller.rc(); HRESULT rc = S_OK; @@ -1335,5 +1344,5 @@ { AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + GuestProcessIterConst it; for (it = mGuestProcessList.begin(); it != mGuestProcessList.end(); it++) @@ -1342,5 +1351,5 @@ break; } - + if (it != mGuestProcessList.end()) {