Index: /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h =================================================================== --- /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h (revision 55587) +++ /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h (revision 55588) @@ -6,5 +6,5 @@ /* - * Copyright (C) 2011-2013 Oracle Corporation + * Copyright (C) 2011-2015 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -21,6 +21,8 @@ #include "ConsoleImpl.h" +#include "Global.h" #include +#include #include @@ -60,46 +62,366 @@ -typedef std::vector GuestEnvironmentArray; -class GuestEnvironment -{ -public: - - int BuildEnvironmentBlock(void **ppvEnv, size_t *pcbEnv, uint32_t *pcEnvVars); - - void Clear(void); - - int CopyFrom(const GuestEnvironmentArray &environment); - - int CopyTo(GuestEnvironmentArray &environment); - - static void FreeEnvironmentBlock(void *pvEnv); - - Utf8Str Get(const Utf8Str &strKey); - - Utf8Str Get(size_t nPos); - - bool Has(const Utf8Str &strKey); - - int Set(const Utf8Str &strKey, const Utf8Str &strValue); - - int Set(const Utf8Str &strPair); - - size_t Size(void); - - int Unset(const Utf8Str &strKey); - -public: - - GuestEnvironment& operator=(const GuestEnvironmentArray &that); - - GuestEnvironment& operator=(const GuestEnvironment &that); - -protected: - - int appendToEnvBlock(const char *pszEnv, void **ppvList, size_t *pcbList, uint32_t *pcEnvVars); - -protected: - - std::map mEnvironment; + +/** + * Wrapper around the RTEnv API, unusable base class. + * + * @remarks Feel free to elevate this class to iprt/cpp/env.h as RTCEnv. + */ +class GuestEnvironmentBase +{ +public: + /** + * Default constructor. + * + * The user must invoke one of the init methods before using the object. + */ + GuestEnvironmentBase(void) + : m_hEnv(NIL_RTENV) + { } + + /** + * Destructor. + */ + virtual ~GuestEnvironmentBase(void) + { + int rc = RTEnvDestroy(m_hEnv); AssertRC(rc); + m_hEnv = NIL_RTENV; + } + + /** + * Initialize this as a normal environment block. + * @returns IPRT status code. + */ + int initNormal(void) + { + AssertReturn(m_hEnv == NIL_RTENV, VERR_WRONG_ORDER); + return RTEnvCreate(&m_hEnv); + } + + /** + * Returns the variable count. + * @return Number of variables. + * @sa RTEnvCountEx + */ + uint32_t count(void) const + { + return RTEnvCountEx(m_hEnv); + } + + /** + * Deletes the environment change record entirely. + * + * The count() method will return zero after this call. + * + * @sa RTEnvReset + */ + void reset(void) + { + int rc = RTEnvReset(m_hEnv); + AssertRC(rc); + } + + /** + * Exports the environment change block as an array of putenv style strings. + * + * + * @returns VINF_SUCCESS or VERR_NO_MEMORY. + * @param pArray The output array. + */ + int queryPutEnvArray(std::vector *pArray) const + { + uint32_t cVars = RTEnvCountEx(m_hEnv); + try + { + pArray->resize(cVars); + for (uint32_t iVar = 0; iVar < cVars; iVar++) + { + const char *psz = RTEnvGetByIndexRawEx(m_hEnv, iVar); + AssertReturn(psz, VERR_INTERNAL_ERROR_3); /* someone is racing us! */ + (*pArray)[iVar] = psz; + } + return VINF_SUCCESS; + } + catch (std::bad_alloc &) + { + return VERR_NO_MEMORY; + } + } + + /** + * Applies an array of putenv style strings. + * + * @returns IPRT status code. + * @param rArray The array with the putenv style strings. + * @sa RTEnvPutEnvEx + */ + int applyPutEnvArray(const std::vector &rArray) + { + size_t cArray = rArray.size(); + for (size_t i = 0; i < cArray; i++) + { + int rc = RTEnvPutEx(m_hEnv, rArray[i].c_str()); + if (RT_FAILURE(rc)) + return rc; + } + return VINF_SUCCESS; + } + + /** + * See RTEnvQueryUtf8Block for details. + * @returns IPRT status code. + * @param ppszzBlock Where to return the block pointer. + * @param pcbBlock Where to optionally return the block size. + * @sa RTEnvQueryUtf8Block + */ + int queryUtf8Block(char **ppszzBlock, size_t *pcbBlock) + { + return RTEnvQueryUtf8Block(m_hEnv, true /*fSorted*/, ppszzBlock, pcbBlock); + } + + /** + * Frees what queryUtf8Block returned, NULL ignored. + * @sa RTEnvFreeUtf8Block + */ + static void freeUtf8Block(char *pszzBlock) + { + return RTEnvFreeUtf8Block(pszzBlock); + } + + /** + * Get an environment variable. + * + * @returns IPRT status code. + * @param rName The variable name. + * @param pValue Where to return the value. + * @sa RTEnvGetEx + */ + int getVariable(const com::Utf8Str &rName, com::Utf8Str *pValue) const + { + size_t cchNeeded; + int rc = RTEnvGetEx(m_hEnv, rName.c_str(), NULL, 0, &cchNeeded); + if ( RT_SUCCESS(rc) + || rc == VERR_BUFFER_OVERFLOW) + { + try + { + pValue->reserve(cchNeeded + 1); + rc = RTEnvGetEx(m_hEnv, rName.c_str(), pValue->mutableRaw(), pValue->capacity(), NULL); + pValue->jolt(); + } + catch (std::bad_alloc &) + { + rc = VERR_NO_STR_MEMORY; + } + } + return rc; + } + + /** + * Set an environment variable. + * + * @returns IPRT status code. + * @param rName The variable name. + * @param rValue The value of the variable. + * @sa RTEnvSetEx + */ + int setVariable(const com::Utf8Str &rName, const com::Utf8Str &rValue) + { + return RTEnvSetEx(m_hEnv, rName.c_str(), rValue.c_str()); + } + + /** + * Unset an environment variable. + * + * @returns IPRT status code. + * @param rName The variable name. + * @sa RTEnvUnsetEx + */ + int unsetVariable(const com::Utf8Str &rName) + { + return RTEnvUnsetEx(m_hEnv, rName.c_str()); + } + +#if 0 +private: + /* No copy operator. */ + GuestEnvironmentBase(const GuestEnvironmentBase &) { throw E_FAIL; } +#else + /** + * Copy constructor. + * @throws HRESULT + */ + GuestEnvironmentBase(const GuestEnvironmentBase &rThat, bool fChangeRecord) + : m_hEnv(NIL_RTENV) + { + int rc = cloneCommon(rThat, fChangeRecord); + if (RT_FAILURE(rc)) + throw (Global::vboxStatusCodeToCOM(rc)); + } +#endif + +protected: + /** + * Common clone/copy method with type conversion abilities. + * + * @returns IPRT status code. + * @param rThat The object to clone. + * @param fChangeRecord Whether the this instance is a change record (true) + * or normal (false) environment. + */ + int cloneCommon(const GuestEnvironmentBase &rThat, bool fChangeRecord) + { + int rc = VINF_SUCCESS; + RTENV hNewEnv = NIL_RTENV; + if (rThat.m_hEnv != NIL_RTENV) + { + if (RTEnvIsChangeRecord(rThat.m_hEnv) == fChangeRecord) + rc = RTEnvClone(&hNewEnv, rThat.m_hEnv); + else + { + /* Need to type convert it. */ + if (fChangeRecord) + rc = RTEnvCreateChangeRecord(&hNewEnv); + else + rc = RTEnvCreate(&hNewEnv); + if (RT_SUCCESS(rc)) + { + rc = RTEnvApplyChanges(hNewEnv, rThat.m_hEnv); + if (RT_FAILURE(rc)) + RTEnvDestroy(hNewEnv); + } + } + + } + if (RT_SUCCESS(rc)) + { + RTEnvDestroy(m_hEnv); + m_hEnv = hNewEnv; + } + return rc; + } + + + /** The environment change record. */ + RTENV m_hEnv; +}; + + +#if 0 /* Not currently used. */ +/** + * Wrapper around the RTEnv API for a normal environment. + */ +class GuestEnvironment : public GuestEnvironmentBase +{ +public: + /** + * Default constructor. + * + * The user must invoke one of the init methods before using the object. + */ + GuestEnvironment(void) + : GuestEnvironmentBase() + { } + + /** + * Copy operator. + * @param rThat The object to copy. + * @throws HRESULT + */ + GuestEnvironment(const GuestEnvironment &rThat) + : GuestEnvironmentBase(rThat, false /*fChangeRecord*/) + { } + + /** + * Copy operator. + * @param rThat The object to copy. + * @throws HRESULT + */ + GuestEnvironment(const GuestEnvironmentBase &rThat) + : GuestEnvironmentBase(rThat, false /*fChangeRecord*/) + { } + + /** + * Initialize this as a normal environment block. + * @returns IPRT status code. + */ + int initNormal(void) + { + AssertReturn(m_hEnv == NIL_RTENV, VERR_WRONG_ORDER); + return RTEnvCreate(&m_hEnv); + } + + /** + * Replaces this environemnt with that in @a rThat. + * + * @returns IPRT status code + * @param rThat The environment to copy. If it's a different type + * we'll convert the data to a normal environment block. + */ + int copy(const GuestEnvironmentBase &rThat) + { + return cloneCommon(rThat, false /*fChangeRecord*/); + } +}; +#endif /* unused */ + + +/** + * Wrapper around the RTEnv API for a environment change record. + * + * This class is used as a record of changes to be applied to a different + * environment block (in VBoxService before launching a new process). + */ +class GuestEnvironmentChanges : public GuestEnvironmentBase +{ +public: + /** + * Default constructor. + * + * The user must invoke one of the init methods before using the object. + */ + GuestEnvironmentChanges(void) + : GuestEnvironmentBase() + { } + + /** + * Copy operator. + * @param rThat The object to copy. + * @throws HRESULT + */ + GuestEnvironmentChanges(const GuestEnvironmentChanges &rThat) + : GuestEnvironmentBase(rThat, true /*fChangeRecord*/) + { } + + /** + * Copy operator. + * @param rThat The object to copy. + * @throws HRESULT + */ + GuestEnvironmentChanges(const GuestEnvironmentBase &rThat) + : GuestEnvironmentBase(rThat, true /*fChangeRecord*/) + { } + + /** + * Initialize this as a environment change record. + * @returns IPRT status code. + */ + int initChangeRecord(void) + { + AssertReturn(m_hEnv == NIL_RTENV, VERR_WRONG_ORDER); + return RTEnvCreateChangeRecord(&m_hEnv); + } + + /** + * Replaces this environemnt with that in @a rThat. + * + * @returns IPRT status code + * @param rThat The environment to copy. If it's a different type + * we'll convert the data to a set of changes. + */ + int copy(const GuestEnvironmentBase &rThat) + { + return cloneCommon(rThat, true /*fChangeRecord*/); + } }; @@ -225,5 +547,6 @@ /** Arguments vector (starting with argument \#0). */ ProcessArguments mArguments; - GuestEnvironment mEnvironment; + /** The process environment change record. */ + GuestEnvironmentChanges mEnvironment; /** Process creation flags. */ uint32_t mFlags; Index: /trunk/src/VBox/Main/include/GuestSessionImpl.h =================================================================== --- /trunk/src/VBox/Main/include/GuestSessionImpl.h (revision 55587) +++ /trunk/src/VBox/Main/include/GuestSessionImpl.h (revision 55588) @@ -391,4 +391,5 @@ public: /** @name Public internal methods. + * @todo r=bird: Most of these are public for no real reason... * @{ */ int i_closeSession(uint32_t uFlags, uint32_t uTimeoutMS, int *pGuestRc); @@ -415,5 +416,4 @@ int i_fsQueryInfoInternal(const Utf8Str &strPath, GuestFsObjData &objData, int *pGuestRc); const GuestCredentials &i_getCredentials(void); - const GuestEnvironment &i_getEnvironment(void); EventSource *i_getEventSource(void) { return mEventSource; } Utf8Str i_getName(void); @@ -441,5 +441,5 @@ int i_startTaskAsync(const Utf8Str &strTaskDesc, GuestSessionTask *pTask, ComObjPtr &pProgress); - int i_queryInfo(void); + int i_determineProtocolVersion(void); int i_waitFor(uint32_t fWaitFlags, ULONG uTimeoutMS, GuestSessionWaitResult_T &waitResult, int *pGuestRc); int i_waitForStatusChange(GuestWaitEvent *pEvent, uint32_t fWaitFlags, uint32_t uTimeoutMS, @@ -470,7 +470,7 @@ /** The session's current status. */ GuestSessionStatus_T mStatus; - /** The session's environment block. Can be - * overwritten/extended by ProcessCreate(Ex). */ - GuestEnvironment mEnvironment; + /** The set of environment changes for the session for use when + * creating new guest processes. */ + GuestEnvironmentChanges mEnvironment; /** Directory objects bound to this session. */ SessionDirectories mDirectories; Index: /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp (revision 55587) +++ /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp (revision 55588) @@ -1,10 +1,9 @@ /* $Id$ */ /** @file - * * Internal helpers/structures for guest control functionality. */ /* - * Copyright (C) 2011-2014 Oracle Corporation + * Copyright (C) 2011-2015 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -37,256 +36,5 @@ #include -/****************************************************************************** - * Structures and Typedefs * - ******************************************************************************/ - -int GuestEnvironment::BuildEnvironmentBlock(void **ppvEnv, size_t *pcbEnv, uint32_t *pcEnvVars) -{ - AssertPtrReturn(ppvEnv, VERR_INVALID_POINTER); - /* Rest is optional. */ - - size_t cbEnv = 0; - uint32_t cEnvVars = 0; - - int rc = VINF_SUCCESS; - - size_t cEnv = mEnvironment.size(); - if (cEnv) - { - std::map::const_iterator itEnv = mEnvironment.begin(); - for (; itEnv != mEnvironment.end() && RT_SUCCESS(rc); itEnv++) - { - char *pszEnv; - if (!RTStrAPrintf(&pszEnv, "%s=%s", itEnv->first.c_str(), itEnv->second.c_str())) - { - rc = VERR_NO_MEMORY; - break; - } - AssertPtr(pszEnv); - rc = appendToEnvBlock(pszEnv, ppvEnv, &cbEnv, &cEnvVars); - RTStrFree(pszEnv); - } - Assert(cEnv == cEnvVars); - } - - if (pcbEnv) - *pcbEnv = cbEnv; - if (pcEnvVars) - *pcEnvVars = cEnvVars; - - return rc; -} - -void GuestEnvironment::Clear(void) -{ - mEnvironment.clear(); -} - -int GuestEnvironment::CopyFrom(const GuestEnvironmentArray &environment) -{ - int rc = VINF_SUCCESS; - - for (GuestEnvironmentArray::const_iterator it = environment.begin(); - it != environment.end() && RT_SUCCESS(rc); - ++it) - { - rc = Set((*it)); - } - - return rc; -} - -int GuestEnvironment::CopyTo(GuestEnvironmentArray &environment) -{ - size_t s = 0; - for (std::map::const_iterator it = mEnvironment.begin(); - it != mEnvironment.end(); - ++it, ++s) - { - environment[s] = Bstr(it->first + "=" + it->second).raw(); - } - - return VINF_SUCCESS; -} - -/* static */ -void GuestEnvironment::FreeEnvironmentBlock(void *pvEnv) -{ - if (pvEnv) - RTMemFree(pvEnv); -} - -Utf8Str GuestEnvironment::Get(size_t nPos) -{ - size_t curPos = 0; - std::map::const_iterator it = mEnvironment.begin(); - for (; it != mEnvironment.end() && curPos < nPos; - ++it, ++curPos) { } - - if (it != mEnvironment.end()) - return Utf8Str(it->first + "=" + it->second); - - return Utf8Str(""); -} - -Utf8Str GuestEnvironment::Get(const Utf8Str &strKey) -{ - std::map ::const_iterator itEnv = mEnvironment.find(strKey); - Utf8Str strRet; - if (itEnv != mEnvironment.end()) - strRet = itEnv->second; - return strRet; -} - -bool GuestEnvironment::Has(const Utf8Str &strKey) -{ - std::map ::const_iterator itEnv = mEnvironment.find(strKey); - return (itEnv != mEnvironment.end()); -} - -int GuestEnvironment::Set(const Utf8Str &strKey, const Utf8Str &strValue) -{ - /** @todo Do some validation using regex. */ - if (strKey.isEmpty()) - return VERR_INVALID_PARAMETER; - - int rc = VINF_SUCCESS; - const char *pszString = strKey.c_str(); - while (*pszString != '\0' && RT_SUCCESS(rc)) - { - if ( !RT_C_IS_ALNUM(*pszString) - && !RT_C_IS_GRAPH(*pszString)) - rc = VERR_INVALID_PARAMETER; - *pszString++; - } - - if (RT_SUCCESS(rc)) - mEnvironment[strKey] = strValue; - - return rc; -} - -int GuestEnvironment::Set(const Utf8Str &strPair) -{ - RTCList listPair = strPair.split("=", RTCString::KeepEmptyParts); - /* Skip completely empty pairs. Note that we still need pairs with a valid - * (set) key and an empty value. */ - if (listPair.size() <= 1) - return VINF_SUCCESS; - - int rc = VINF_SUCCESS; - size_t p = 0; - while (p < listPair.size() && RT_SUCCESS(rc)) - { - Utf8Str strKey = listPair.at(p++); - if ( strKey.isEmpty() - || strKey.equals("=")) /* Skip pairs with empty keys (e.g. "=FOO"). */ - { - break; - } - Utf8Str strValue; - if (p < listPair.size()) /* Does the list also contain a value? */ - strValue = listPair.at(p++); - -#ifdef DEBUG - LogFlowFunc(("strKey=%s, strValue=%s\n", - strKey.c_str(), strValue.c_str())); -#endif - rc = Set(strKey, strValue); - } - - return rc; -} - -size_t GuestEnvironment::Size(void) -{ - return mEnvironment.size(); -} - -int GuestEnvironment::Unset(const Utf8Str &strKey) -{ - std::map ::iterator itEnv = mEnvironment.find(strKey); - if (itEnv != mEnvironment.end()) - { - mEnvironment.erase(itEnv); - return VINF_SUCCESS; - } - - return VERR_NOT_FOUND; -} - -GuestEnvironment& GuestEnvironment::operator=(const GuestEnvironmentArray &that) -{ - CopyFrom(that); - return *this; -} - -GuestEnvironment& GuestEnvironment::operator=(const GuestEnvironment &that) -{ - for (std::map::const_iterator it = that.mEnvironment.begin(); - it != that.mEnvironment.end(); - ++it) - { - mEnvironment[it->first] = it->second; - } - - return *this; -} - -/** - * Appends environment variables to the environment block. - * - * Each var=value pair is separated by the null character ('\\0'). The whole - * block will be stored in one blob and disassembled on the guest side later to - * fit into the HGCM param structure. - * - * @returns VBox status code. - * - * @param pszEnvVar The environment variable=value to append to the - * environment block. - * @param ppvList This is actually a pointer to a char pointer - * variable which keeps track of the environment block - * that we're constructing. - * @param pcbList Pointer to the variable holding the current size of - * the environment block. (List is a misnomer, go - * ahead a be confused.) - * @param pcEnvVars Pointer to the variable holding count of variables - * stored in the environment block. - */ -int GuestEnvironment::appendToEnvBlock(const char *pszEnv, void **ppvList, size_t *pcbList, uint32_t *pcEnvVars) -{ - int rc = VINF_SUCCESS; - size_t cchEnv = strlen(pszEnv); Assert(cchEnv >= 2); - if (*ppvList) - { - size_t cbNewLen = *pcbList + cchEnv + 1; /* Include zero termination. */ - char *pvTmp = (char *)RTMemRealloc(*ppvList, cbNewLen); - if (pvTmp == NULL) - rc = VERR_NO_MEMORY; - else - { - memcpy(pvTmp + *pcbList, pszEnv, cchEnv); - pvTmp[cbNewLen - 1] = '\0'; /* Add zero termination. */ - *ppvList = (void **)pvTmp; - } - } - else - { - char *pszTmp; - if (RTStrAPrintf(&pszTmp, "%s", pszEnv) >= 0) - { - *ppvList = (void **)pszTmp; - /* Reset counters. */ - *pcEnvVars = 0; - *pcbList = 0; - } - } - if (RT_SUCCESS(rc)) - { - *pcbList += cchEnv + 1; /* Include zero termination. */ - *pcEnvVars += 1; /* Increase env variable count. */ - } - return rc; -} + int GuestFsObjData::FromLs(const GuestProcessStreamBlock &strmBlk) Index: /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp (revision 55587) +++ /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp (revision 55588) @@ -304,5 +304,5 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - mData.mProcess.mEnvironment.CopyTo(aEnvironment); + mData.mProcess.mEnvironment.queryPutEnvArray(&aEnvironment); return S_OK; #endif /* VBOX_WITH_GUEST_CONTROL */ @@ -1054,8 +1054,8 @@ /* Prepare environment. */ - void *pvEnv = NULL; - size_t cbEnv = 0; + size_t cbEnvBlock; + char *pszzEnvBlock; if (RT_SUCCESS(vrc)) - vrc = mData.mProcess.mEnvironment.BuildEnvironmentBlock(&pvEnv, &cbEnv, NULL /* cEnv */); + vrc = mData.mProcess.mEnvironment.queryUtf8Block(&pszzEnvBlock, &cbEnvBlock); if (RT_SUCCESS(vrc)) @@ -1065,12 +1065,11 @@ int i = 0; paParms[i++].setUInt32(pEvent->ContextID()); - paParms[i++].setPointer((void*)mData.mProcess.mExecutable.c_str(), - (ULONG)mData.mProcess.mExecutable.length() + 1); + paParms[i++].setCppString(mData.mProcess.mExecutable); paParms[i++].setUInt32(mData.mProcess.mFlags); paParms[i++].setUInt32((uint32_t)mData.mProcess.mArguments.size()); - paParms[i++].setPointer((void*)pszArgs, (uint32_t)cbArgs); - paParms[i++].setUInt32((uint32_t)mData.mProcess.mEnvironment.Size()); - paParms[i++].setUInt32((uint32_t)cbEnv); - paParms[i++].setPointer((void*)pvEnv, (uint32_t)cbEnv); + paParms[i++].setPointer(pszArgs, (uint32_t)cbArgs); + paParms[i++].setUInt32(mData.mProcess.mEnvironment.count()); + paParms[i++].setUInt32((uint32_t)cbEnvBlock); + paParms[i++].setPointer(pszzEnvBlock, (uint32_t)cbEnvBlock); if (uProtocol < 2) { @@ -1078,6 +1077,6 @@ * call. In newer protocols these credentials are part of the opened guest * session, so not needed anymore here. */ - paParms[i++].setPointer((void*)sessionCreds.mUser.c_str(), (ULONG)sessionCreds.mUser.length() + 1); - paParms[i++].setPointer((void*)sessionCreds.mPassword.c_str(), (ULONG)sessionCreds.mPassword.length() + 1); + paParms[i++].setCppString(sessionCreds.mUser); + paParms[i++].setCppString(sessionCreds.mPassword); } /* @@ -1098,5 +1097,5 @@ paParms[i++].setUInt32(1); /* The actual CPU affinity blocks. */ - paParms[i++].setPointer((void*)&mData.mProcess.mAffinity, sizeof(mData.mProcess.mAffinity)); + paParms[i++].setPointer((void *)&mData.mProcess.mAffinity, sizeof(mData.mProcess.mAffinity)); } @@ -1109,7 +1108,8 @@ AssertRC(rc2); } - } - - GuestEnvironment::FreeEnvironmentBlock(pvEnv); + + mData.mProcess.mEnvironment.freeUtf8Block(pszzEnvBlock); + } + if (pszArgs) RTStrFree(pszArgs); Index: /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp (revision 55587) +++ /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp (revision 55588) @@ -183,4 +183,7 @@ AssertPtrReturn(pGuest, VERR_INVALID_POINTER); + /* + * Initialize our data members from the input. + */ mParent = pGuest; @@ -198,70 +201,65 @@ mData.mCredentials.mDomain = guestCreds.mDomain; + /* Initialize the remainder of the data. */ mData.mRC = VINF_SUCCESS; mData.mStatus = GuestSessionStatus_Undefined; mData.mNumObjects = 0; - - HRESULT hr; - - int rc = i_queryInfo(); + int rc = mData.mEnvironment.initNormal(); if (RT_SUCCESS(rc)) { - hr = unconst(mEventSource).createObject(); - if (FAILED(hr)) - rc = VERR_NO_MEMORY; - else - { + rc = RTCritSectInit(&mWaitEventCritSect); + AssertRC(rc); + } + if (RT_SUCCESS(rc)) + rc = i_determineProtocolVersion(); + if (RT_SUCCESS(rc)) + { + /* + * + */ + HRESULT hr = unconst(mEventSource).createObject(); + if (SUCCEEDED(hr)) hr = mEventSource->init(); - if (FAILED(hr)) - rc = VERR_COM_UNEXPECTED; - } - } - - if (RT_SUCCESS(rc)) - { - try - { - GuestSessionListener *pListener = new GuestSessionListener(); - ComObjPtr thisListener; - hr = thisListener.createObject(); - if (SUCCEEDED(hr)) - hr = thisListener->init(pListener, this); - - if (SUCCEEDED(hr)) + if (SUCCEEDED(hr)) + { + try { - com::SafeArray eventTypes; - eventTypes.push_back(VBoxEventType_OnGuestSessionStateChanged); - hr = mEventSource->RegisterListener(thisListener, - ComSafeArrayAsInParam(eventTypes), - TRUE /* Active listener */); + GuestSessionListener *pListener = new GuestSessionListener(); + ComObjPtr thisListener; + hr = thisListener.createObject(); + if (SUCCEEDED(hr)) + hr = thisListener->init(pListener, this); if (SUCCEEDED(hr)) { - mLocalListener = thisListener; - - rc = RTCritSectInit(&mWaitEventCritSect); - AssertRC(rc); + com::SafeArray eventTypes; + eventTypes.push_back(VBoxEventType_OnGuestSessionStateChanged); + hr = mEventSource->RegisterListener(thisListener, + ComSafeArrayAsInParam(eventTypes), + TRUE /* Active listener */); + if (SUCCEEDED(hr)) + { + mLocalListener = thisListener; + + /* + * Mark this object as operational and return success. + */ + autoInitSpan.setSucceeded(); + LogFlowThisFunc(("mName=%s mID=%RU32 mIsInternal=%RTbool rc=VINF_SUCCESS\n", + mData.mSession.mName.c_str(), mData.mSession.mID, mData.mSession.mIsInternal)); + return VINF_SUCCESS; + } } - else - rc = VERR_COM_UNEXPECTED; } - else - rc = VERR_COM_UNEXPECTED; - } - catch(std::bad_alloc &) - { - rc = VERR_NO_MEMORY; - } - } - - if (RT_SUCCESS(rc)) - { - /* Confirm a successful initialization when it's the case. */ - autoInitSpan.setSucceeded(); - } - else - autoInitSpan.setFailed(); - - LogFlowThisFunc(("mName=%s, mID=%RU32, mIsInternal=%RTbool, rc=%Rrc\n", - mData.mSession.mName.c_str(), mData.mSession.mID, mData.mSession.mIsInternal, rc)); + catch (std::bad_alloc &) + { + hr = E_OUTOFMEMORY; + } + } + rc = Global::vboxStatusCodeFromCOM(hr); + } + + autoInitSpan.setFailed(); + LogThisFunc(("Failed! mName=%s mID=%RU32 mIsInternal=%RTbool => rc=%Rrc\n", + mData.mSession.mName.c_str(), mData.mSession.mID, mData.mSession.mIsInternal, rc)); return rc; #endif /* VBOX_WITH_GUEST_CONTROL */ @@ -322,4 +320,6 @@ mData.mProcesses.clear(); + mData.mEnvironment.reset(); + AssertMsg(mData.mNumObjects == 0, ("mNumObjects=%RU32 when it should be 0\n", mData.mNumObjects)); @@ -471,15 +471,8 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - size_t cEnvVars = mData.mEnvironment.Size(); - aEnvironment.resize(cEnvVars); - - LogFlowThisFunc(("[%s]: cEnvVars=%RU32\n", - mData.mSession.mName.c_str(), cEnvVars)); - - for (size_t i = 0; i < cEnvVars; i++) - aEnvironment[i] = mData.mEnvironment.Get(i); - - LogFlowThisFuncLeave(); - return S_OK; + int vrc = mData.mEnvironment.queryPutEnvArray(&aEnvironment); + + LogFlowFuncLeaveRC(vrc); + return Global::vboxStatusCodeToCOM(vrc); #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -494,12 +487,9 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - int rc = VINF_SUCCESS; - for (size_t i = 0; i < aEnvironment.size() && RT_SUCCESS(rc); ++i) - if (!aEnvironment[i].isEmpty()) /* Silently skip empty entries. */ - rc = mData.mEnvironment.Set(aEnvironment[i]); - - HRESULT hr = RT_SUCCESS(rc) ? S_OK : VBOX_E_IPRT_ERROR; - LogFlowFuncLeaveRC(hr); - return hr; + mData.mEnvironment.reset(); + int vrc = mData.mEnvironment.applyPutEnvArray(aEnvironment); + + LogFlowFuncLeaveRC(vrc); + return Global::vboxStatusCodeToCOM(vrc); #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -1414,9 +1404,4 @@ { return mData.mCredentials; -} - -const GuestEnvironment& GuestSession::i_getEnvironment(void) -{ - return mData.mEnvironment; } @@ -2118,45 +2103,40 @@ /** - * Queries/collects information prior to establishing a guest session. - * This is necessary to know which guest control protocol version to use, - * among other things (later). + * Determines the protocol version (sets mData.mProtocolVersion). + * + * This is called from the init method prior to to establishing a guest + * session. * * @return IPRT status code. */ -int GuestSession::i_queryInfo(void) +int GuestSession::i_determineProtocolVersion(void) { /* - * Try querying the guest control protocol version running on the guest. - * This is done using the Guest Additions version + * We currently do this based on the reported guest additions version, + * ASSUMING that VBoxService and VBoxDrv are at the same version. */ ComObjPtr pGuest = mParent; - Assert(!pGuest.isNull()); - - uint32_t uVerAdditions = pGuest->i_getAdditionsVersion(); - uint32_t uVBoxMajor = VBOX_FULL_VERSION_GET_MAJOR(uVerAdditions); - uint32_t uVBoxMinor = VBOX_FULL_VERSION_GET_MINOR(uVerAdditions); - -#ifdef DEBUG_andy - /* Hardcode the to-used protocol version; nice for testing side effects. */ - mData.mProtocolVersion = 2; -#else - mData.mProtocolVersion = ( - /* VBox 5.0 and up. */ - uVBoxMajor >= 5 - /* VBox 4.3 and up. */ - || (uVBoxMajor == 4 && uVBoxMinor >= 3)) - ? 2 /* Guest control 2.0. */ - : 1; /* Legacy guest control (VBox < 4.3). */ - /* Build revision is ignored. */ -#endif - - LogFlowThisFunc(("uVerAdditions=%RU32 (%RU32.%RU32), mProtocolVersion=%RU32\n", - uVerAdditions, uVBoxMajor, uVBoxMinor, mData.mProtocolVersion)); - - /* Tell the user but don't bitch too often. */ - /** @todo Find a bit nicer text. */ + AssertReturn(!pGuest.isNull(), VERR_NOT_SUPPORTED); + uint32_t uGaVersion = pGuest->i_getAdditionsVersion(); + + /* Everyone supports version one, if they support anything at all. */ + mData.mProtocolVersion = 1; + + /* Guest control 2.0 was introduced with 4.3.0. */ + if (uGaVersion >= VBOX_FULL_VERSION_MAKE(4,3,0)) + mData.mProtocolVersion = 2; /* Guest control 2.0. */ + + LogFlowThisFunc(("uGaVersion=%u.%u.%u => mProtocolVersion=%u\n", + VBOX_FULL_VERSION_GET_MAJOR(uGaVersion), VBOX_FULL_VERSION_GET_MINOR(uGaVersion), + VBOX_FULL_VERSION_GET_BUILD(uGaVersion), mData.mProtocolVersion)); + + /* + * Inform the user about outdated guest additions (VM release log). + */ if (mData.mProtocolVersion < 2) - LogRelMax(3, (tr("Warning: Guest Additions are older (%ld.%ld) than host capabilities for guest control, please upgrade them. Using protocol version %ld now\n"), - uVBoxMajor, uVBoxMinor, mData.mProtocolVersion)); + LogRelMax(3, (tr("Warning: Guest Additions v%u.%u.%u only supports the older guest control protocol version %u.\n" + " Please upgrade GAs to the current version to get full guest control capabilities.\n"), + VBOX_FULL_VERSION_GET_MAJOR(uGaVersion), VBOX_FULL_VERSION_GET_MINOR(uGaVersion), + VBOX_FULL_VERSION_GET_BUILD(uGaVersion), mData.mProtocolVersion)); return VINF_SUCCESS; @@ -2924,4 +2904,5 @@ } +/** @todo remove this it duplicates the 'environment' attribute. */ HRESULT GuestSession::environmentClear() { @@ -2933,5 +2914,5 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - mData.mEnvironment.Clear(); + mData.mEnvironment.reset(); LogFlowThisFuncLeave(); @@ -2940,4 +2921,8 @@ } +/** @todo Remove this because the interface isn't suitable for returning 'unset' + * or empty values, and it can easily be misunderstood. Besides there + * is hardly a usecase for it as long as it just works on + * environment changes and there is the 'environment' attribute. */ HRESULT GuestSession::environmentGet(const com::Utf8Str &aName, com::Utf8Str &aValue) { @@ -2947,13 +2932,22 @@ LogFlowThisFuncEnter(); - if (RT_UNLIKELY(aName.c_str() == NULL) || *(aName.c_str()) == '\0') - return setError(E_INVALIDARG, tr("No value name specified")); - - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - - aValue = mData.mEnvironment.Get(aName); + HRESULT hrc; + if (RT_LIKELY(aName.isNotEmpty())) + { + if (RT_LIKELY(strchr(aName.c_str(), '=') == NULL)) + { + AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); + + mData.mEnvironment.getVariable(aName, &aValue); + hrc = S_OK; + } + else + hrc = setError(E_INVALIDARG, tr("The equal char is not allowed in environment variable names")); + } + else + hrc = setError(E_INVALIDARG, tr("No variable name specified")); LogFlowThisFuncLeave(); - return S_OK; + return hrc; #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -2966,14 +2960,24 @@ LogFlowThisFuncEnter(); - if (RT_UNLIKELY((aName.c_str() == NULL) || *(aName.c_str()) == '\0')) - return setError(E_INVALIDARG, tr("No value name specified")); - - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - int rc = mData.mEnvironment.Set(aName, aValue); - - HRESULT hr = RT_SUCCESS(rc) ? S_OK : VBOX_E_IPRT_ERROR; - LogFlowFuncLeaveRC(hr); - return hr; + HRESULT hrc; + if (RT_LIKELY(aName.isNotEmpty())) + { + if (RT_LIKELY(strchr(aName.c_str(), '=') == NULL)) + { + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + int vrc = mData.mEnvironment.setVariable(aName, aValue); + if (RT_SUCCESS(vrc)) + hrc = S_OK; + else + hrc = setErrorVrc(vrc); + } + else + hrc = setError(E_INVALIDARG, tr("The equal char is not allowed in environment variable names")); + } + else + hrc = setError(E_INVALIDARG, tr("No variable name specified")); + + LogFlowThisFuncLeave(); + return hrc; #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -2985,11 +2989,24 @@ #else LogFlowThisFuncEnter(); - - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - mData.mEnvironment.Unset(aName); + HRESULT hrc; + if (RT_LIKELY(aName.isNotEmpty())) + { + if (RT_LIKELY(strchr(aName.c_str(), '=') == NULL)) + { + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + int vrc = mData.mEnvironment.unsetVariable(aName); + if (RT_SUCCESS(vrc)) + hrc = S_OK; + else + hrc = setErrorVrc(vrc); + } + else + hrc = setError(E_INVALIDARG, tr("The equal char is not allowed in environment variable names")); + } + else + hrc = setError(E_INVALIDARG, tr("No variable name specified")); LogFlowThisFuncLeave(); - return S_OK; + return hrc; #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -3338,4 +3355,9 @@ LogFlowThisFuncEnter(); + /** @todo r=bird: Check input better? aPriority is passed on to the guest + * without any validation. Flags not existing in this vbox version are + * ignored, potentially doing something entirely different than what the + * caller had in mind. */ + /* * Must have an executable to execute. If none is given, we try use the @@ -3369,26 +3391,11 @@ procInfo.mArguments.push_back(aArguments[i]); - /* - * Create the process environment: - * - Apply the session environment in a first step, and - * - Apply environment variables specified by this call to - * have the chance of overwriting/deleting session entries. - */ - procInfo.mEnvironment = mData.mEnvironment; /* Apply original session environment. */ - - int rc = VINF_SUCCESS; - if (aEnvironment.size()) - for (size_t i = 0; i < aEnvironment.size() && RT_SUCCESS(rc); i++) - { - /** @todo r=bird: What ARE you trying to do here??? The documentation is crystal - * clear on that each entry contains ONE pair, however, - * GuestEnvironment::Set(const Utf8Str &) here will split up the input - * into any number of pairs, from what I can tell. Such that for e.g. - * "VBOX_LOG_DEST=file=/tmp/foobared.log" becomes "VBOX_LOG_DEST=file" - * and "/tmp/foobared.log" - which I obviously don't want! */ - rc = procInfo.mEnvironment.Set(aEnvironment[i]); - } - - if (RT_SUCCESS(rc)) + /* Combine the environment changes associated with the ones passed in by + the caller, giving priority to the latter. The changes are putenv style + and will be applied to the standard environment for the guest user. */ + int vrc = procInfo.mEnvironment.copy(mData.mEnvironment); + if (RT_SUCCESS(vrc)) + vrc = procInfo.mEnvironment.applyPutEnvArray(aEnvironment); + if (RT_SUCCESS(vrc)) { /* Convert the flag array into a mask. */ @@ -3411,47 +3418,36 @@ */ ComObjPtr pProcess; - rc = i_processCreateExInternal(procInfo, pProcess); - if (RT_SUCCESS(rc)) + vrc = i_processCreateExInternal(procInfo, pProcess); + if (RT_SUCCESS(vrc)) { /* Return guest session to the caller. */ - HRESULT hr2 = pProcess.queryInterfaceTo(aGuestProcess.asOutParam()); - if (SUCCEEDED(hr2)) + hr = pProcess.queryInterfaceTo(aGuestProcess.asOutParam()); + if (SUCCEEDED(hr)) { /* * Start the process. */ - rc = pProcess->i_startProcessAsync(); - if (RT_FAILURE(rc)) + vrc = pProcess->i_startProcessAsync(); + if (RT_SUCCESS(vrc)) { - /** @todo r=bird: What happens to the interface that *aGuestProcess points to - * now? Looks like a leak or an undocument hack of sorts... */ + LogFlowFuncLeaveRC(vrc); + return S_OK; } + + hr = setErrorVrc(vrc, tr("Failed to start guest process: %Rrc"), vrc); + /** @todo r=bird: What happens to the interface that *aGuestProcess points to + * now? Looks like a leak or an undocument hack of sorts... */ } - else - rc = VERR_COM_OBJECT_NOT_FOUND; - } - } - - /** @todo you're better off doing this is 'else if (rc == xxx') statements, - * since there is just one place where you'll get - * VERR_MAX_PROCS_REACHED in the above code. */ - if (RT_FAILURE(rc)) - { - switch (rc) - { - case VERR_MAX_PROCS_REACHED: - hr = setError(VBOX_E_IPRT_ERROR, tr("Maximum number of concurrent guest processes per session (%ld) reached"), - VBOX_GUESTCTRL_MAX_OBJECTS); - break; - - /** @todo Add more errors here. */ - - default: - hr = setError(VBOX_E_IPRT_ERROR, tr("Could not create guest process, rc=%Rrc"), rc); - break; - } - } - - LogFlowFuncLeaveRC(rc); + } + else if (vrc == VERR_MAX_PROCS_REACHED) + hr = setErrorVrc(vrc, tr("Maximum number of concurrent guest processes per session (%u) reached"), + VBOX_GUESTCTRL_MAX_OBJECTS); + else + hr = setErrorVrc(vrc, tr("Failed to create guest process object: %Rrc"), vrc); + } + else + hr = setErrorVrc(vrc, tr("Failed to set up the environment: %Rrc"), vrc); + + LogFlowFuncLeaveRC(vrc); return hr; #endif /* VBOX_WITH_GUEST_CONTROL */