Index: /trunk/src/VBox/HostServices/GuestProperties/service.cpp =================================================================== --- /trunk/src/VBox/HostServices/GuestProperties/service.cpp (revision 44154) +++ /trunk/src/VBox/HostServices/GuestProperties/service.cpp (revision 44155) @@ -139,4 +139,6 @@ /** The function that was requested */ uint32_t mFunction; + /** Number of call parameters. */ + uint32_t mParmsCnt; /** The call parameters */ VBOXHGCMSVCPARM *mParms; @@ -145,10 +147,10 @@ /** The standard constructor */ - GuestCall() : u32ClientId(0), mFunction(0) {} + GuestCall(void) : u32ClientId(0), mFunction(0), mParmsCnt(0) {} /** The normal constructor */ GuestCall(uint32_t aClientId, VBOXHGCMCALLHANDLE aHandle, uint32_t aFunction, - VBOXHGCMSVCPARM aParms[], int aRc) - : u32ClientId(aClientId), mHandle(aHandle), mFunction(aFunction), mParms(aParms), - mRc(aRc) {} + uint32_t aParmsCnt, VBOXHGCMSVCPARM aParms[], int aRc) + : u32ClientId(aClientId), mHandle(aHandle), mFunction(aFunction), + mParmsCnt(aParmsCnt), mParms(aParms), mRc(aRc) {} }; /** The guest call list type */ @@ -171,5 +173,6 @@ /** The number of properties. */ unsigned mcProperties; - /** The list of property changes for guest notifications */ + /** The list of property changes for guest notifications; + * only used for timestamp tracking in notifications at the moment */ PropertyList mGuestNotifications; /** The list of outstanding guest notification calls */ @@ -376,6 +379,6 @@ int getOldNotificationInternal(const char *pszPattern, uint64_t u64Timestamp, Property *pProp); - int getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop); - void doNotifications(const char *pszProperty, uint64_t u64Timestamp); + int getNotificationWriteOut(uint32_t cParms, VBOXHGCMSVCPARM paParms[], Property prop); + int doNotifications(const char *pszProperty, uint64_t u64Timestamp); int notifyHost(const char *pszName, const char *pszValue, uint64_t u64Timestamp, const char *pszFlags); @@ -702,8 +705,10 @@ else if (mcProperties < MAX_PROPS) { - /* Create a new string space record. */ - pProp = new Property(pcszName, pcszValue, u64TimeNano, fFlags); - if (pProp) + try { + /* Create a new string space record. */ + pProp = new Property(pcszName, pcszValue, u64TimeNano, fFlags); + AssertPtr(pProp); + if (RTStrSpaceInsert(&mhProperties, &pProp->mStrCore)) mcProperties++; @@ -712,9 +717,12 @@ AssertFailed(); delete pProp; - rc = VERR_INTERNAL_ERROR_3; + + rc = VERR_ALREADY_EXISTS; } } - else + catch (std::bad_alloc) + { rc = VERR_NO_MEMORY; + } } else @@ -722,13 +730,14 @@ /* - * Send a notification to the host and return. + * Send a notification to the guest and host and return. */ // if (isGuest) /* Notify the host even for properties that the host // * changed. Less efficient, but ensures consistency. */ - doNotifications(pcszName, u64TimeNano); - Log2(("Set string %s, rc=%Rrc, value=%s\n", pcszName, rc, pcszValue)); - } - - LogFlowThisFunc(("rc = %Rrc (%s = %s)\n", rc, pcszName, pcszValue)); + int rc2 = doNotifications(pcszName, u64TimeNano); + if (RT_SUCCESS(rc)) + rc = rc2; + } + + LogFlowThisFunc(("%s=%s, rc=%Rrc\n", pcszName, pcszValue, rc)); return rc; } @@ -764,5 +773,5 @@ if (RT_FAILURE(rc)) { - LogFlowThisFunc(("rc = %Rrc\n", rc)); + LogFlowThisFunc(("rc=%Rrc\n", rc)); return rc; } @@ -788,8 +797,10 @@ // if (isGuest) /* Notify the host even for properties that the host // * changed. Less efficient, but ensures consistency. */ - doNotifications(pcszName, u64Timestamp); - } - - LogFlowThisFunc(("rc = %Rrc (%s)\n", rc, pcszName)); + int rc2 = doNotifications(pcszName, u64Timestamp); + if (RT_SUCCESS(rc)) + rc = rc2; + } + + LogFlowThisFunc(("%s: rc=%Rrc\n", pcszName, rc)); return rc; } @@ -968,7 +979,8 @@ /** Helper query used by getNotification */ -int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop) -{ - int rc = VINF_SUCCESS; +int Service::getNotificationWriteOut(uint32_t cParms, VBOXHGCMSVCPARM paParms[], Property prop) +{ + AssertReturn(cParms == 4, VERR_INVALID_PARAMETER); /* Basic sanity checking. */ + /* Format the data to write to the buffer. */ std::string buffer; @@ -976,5 +988,6 @@ char *pchBuf; uint32_t cbBuf; - rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf); + + int rc = paParms[2].getBuffer((void **)&pchBuf, &cbBuf); if (RT_SUCCESS(rc)) { @@ -1015,6 +1028,6 @@ * @throws can throw std::bad_alloc */ -int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, - VBOXHGCMSVCPARM paParms[]) +int Service::getNotification(uint32_t u32ClientId, VBOXHGCMCALLHANDLE callHandle, + uint32_t cParms, VBOXHGCMSVCPARM paParms[]) { int rc = VINF_SUCCESS; @@ -1035,7 +1048,7 @@ ) rc = VERR_INVALID_PARAMETER; + if (RT_SUCCESS(rc)) - LogFlow((" pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns, - u64Timestamp)); + LogFlow(("pszPatterns=%s, u64Timestamp=%llu\n", pszPatterns, u64Timestamp)); /* @@ -1046,43 +1059,48 @@ if (RT_SUCCESS(rc) && u64Timestamp != 0) rc = getOldNotification(pszPatterns, u64Timestamp, &prop); - if (RT_SUCCESS(rc) && prop.isNull()) - { + if (RT_SUCCESS(rc)) + { + if (prop.isNull()) + { + /* + * Check if the client already had the same request. + * Complete the old request with an error in this case. + * Protection against clients, which cancel and resubmits requests. + */ + CallList::iterator it = mGuestWaiters.begin(); + while (it != mGuestWaiters.end()) + { + const char *pszPatternsExisting; + uint32_t cchPatternsExisting; + int rc3 = it->mParms[0].getString(&pszPatternsExisting, &cchPatternsExisting); + + if ( RT_SUCCESS(rc3) + && u32ClientId == it->u32ClientId + && RTStrCmp(pszPatterns, pszPatternsExisting) == 0) + { + /* Complete the old request. */ + mpHelpers->pfnCallComplete(it->mHandle, VERR_INTERRUPTED); + it = mGuestWaiters.erase(it); + } + else + ++it; + } + + mGuestWaiters.push_back(GuestCall(u32ClientId, callHandle, GET_NOTIFICATION, + cParms, paParms, rc)); + rc = VINF_HGCM_ASYNC_EXECUTE; + } /* - * Check if the client already had the same request. - * Complete the old request with an error in this case. - * Protection against clients, which cancel and resubmits requests. + * Otherwise reply at once with the enqueued notification we found. */ - CallList::iterator it = mGuestWaiters.begin(); - while (it != mGuestWaiters.end()) - { - const char *pszPatternsExisting; - uint32_t cchPatternsExisting; - int rc3 = it->mParms[0].getString(&pszPatternsExisting, &cchPatternsExisting); - - if ( RT_SUCCESS(rc3) - && u32ClientId == it->u32ClientId - && RTStrCmp(pszPatterns, pszPatternsExisting) == 0) - { - /* Complete the old request. */ - mpHelpers->pfnCallComplete(it->mHandle, VERR_INTERRUPTED); - it = mGuestWaiters.erase(it); - } - else - ++it; - } - - mGuestWaiters.push_back(GuestCall(u32ClientId, callHandle, GET_NOTIFICATION, - paParms, rc)); - rc = VINF_HGCM_ASYNC_EXECUTE; - } - /* - * Otherwise reply at once with the enqueued notification we found. - */ - else - { - int rc2 = getNotificationWriteOut(paParms, prop); - if (RT_FAILURE(rc2)) - rc = rc2; - } + else + { + int rc2 = getNotificationWriteOut(cParms, paParms, prop); + if (RT_SUCCESS(rc)) + rc = rc2; + } + } + + LogFlowThisFunc(("returning rc=%Rrc\n", rc)); return rc; } @@ -1097,8 +1115,8 @@ * @thread HGCM service */ -void Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp) -{ - AssertPtrReturnVoid(pszProperty); - LogFlowThisFunc (("pszProperty=%s, u64Timestamp=%llu\n", pszProperty, u64Timestamp)); +int Service::doNotifications(const char *pszProperty, uint64_t u64Timestamp) +{ + AssertPtrReturn(pszProperty, VERR_INVALID_POINTER); + LogFlowThisFunc(("pszProperty=%s, u64Timestamp=%llu\n", pszProperty, u64Timestamp)); /* Ensure that our timestamp is different to the last one. */ if ( !mGuestNotifications.empty() @@ -1122,6 +1140,6 @@ } - /* Release waiters if applicable and add the event to the queue for - * guest notifications */ + /* Release guest waiters if applicable and add the event + * to the queue for guest notifications */ int rc = VINF_SUCCESS; try @@ -1136,5 +1154,5 @@ { GuestCall curCall = *it; - int rc2 = getNotificationWriteOut(curCall.mParms, prop); + int rc2 = getNotificationWriteOut(curCall.mParmsCnt, curCall.mParms, prop); if (RT_SUCCESS(rc2)) rc2 = curCall.mRc; @@ -1145,5 +1163,11 @@ ++it; } + mGuestNotifications.push_back(prop); + + /** @todo r=andy This list does not have a purpose but for tracking + * the timestamps ... + if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS) + mGuestNotifications.pop_front(); } catch (std::bad_alloc) @@ -1151,33 +1175,34 @@ rc = VERR_NO_MEMORY; } - if (mGuestNotifications.size() > MAX_GUEST_NOTIFICATIONS) - mGuestNotifications.pop_front(); - - /* - * Host notifications - first case: if the property exists then send its - * current value - */ - if (pProp && mpfnHostCallback != NULL) - { - char szFlags[MAX_FLAGS_LEN]; - /* Send out a host notification */ - const char *pszValue = prop.mValue.c_str(); - if (RT_SUCCESS(rc)) + + if ( RT_SUCCESS(rc) + && mpfnHostCallback) + { + /* + * Host notifications - first case: if the property exists then send its + * current value + */ + if (pProp) + { + char szFlags[MAX_FLAGS_LEN]; + /* Send out a host notification */ + const char *pszValue = prop.mValue.c_str(); rc = writeFlags(prop.mFlags, szFlags); - if (RT_SUCCESS(rc)) - rc = notifyHost(pszProperty, pszValue, u64Timestamp, szFlags); - } - - /* - * Host notifications - second case: if the property does not exist then - * send the host an empty value - */ - if (!pProp && mpfnHostCallback != NULL) - { - /* Send out a host notification */ - if (RT_SUCCESS(rc)) + if (RT_SUCCESS(rc)) + rc = notifyHost(pszProperty, pszValue, u64Timestamp, szFlags); + } + /* + * Host notifications - second case: if the property does not exist then + * send the host an empty value + */ + else + { + /* Send out a host notification */ rc = notifyHost(pszProperty, "", u64Timestamp, ""); - } - LogFlowThisFunc(("returning\n")); + } + } + + LogFlowThisFunc(("returning rc=%Rrc\n", rc)); + return rc; } @@ -1201,8 +1226,8 @@ HostCallbackData.u64Timestamp = u64Timestamp; HostCallbackData.pcszFlags = pszFlags; - int rc = mpfnHostCallback (mpvHostData, 0 /*u32Function*/, - (void *)(&HostCallbackData), - sizeof(HostCallbackData)); - LogFlowFunc (("returning %Rrc\n", rc)); + int rc = mpfnHostCallback(mpvHostData, 0 /*u32Function*/, + (void *)(&HostCallbackData), + sizeof(HostCallbackData)); + LogFlowFunc(("returning rc=%Rrc\n", rc)); return rc; }