Index: /trunk/src/VBox/Main/include/MachineImpl.h =================================================================== --- /trunk/src/VBox/Main/include/MachineImpl.h (revision 44166) +++ /trunk/src/VBox/Main/include/MachineImpl.h (revision 44167) @@ -239,6 +239,4 @@ */ struct GuestProperty { - /** Property name */ - Utf8Str strName; /** Property value */ Utf8Str strValue; @@ -291,6 +289,6 @@ DragAndDropMode_T mDragAndDropMode; - typedef std::list GuestPropertyList; - GuestPropertyList mGuestProperties; + typedef std::map GuestPropertyMap; + GuestPropertyMap mGuestProperties; Utf8Str mGuestPropertyNotificationPatterns; Index: /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 44166) +++ /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 44167) @@ -1698,6 +1698,6 @@ AssertReturn(sizeof(HOSTCALLBACKDATA) == cbParms, VERR_INVALID_PARAMETER); AssertReturn(HOSTCALLBACKMAGIC == pCBData->u32Magic, VERR_INVALID_PARAMETER); - Log5(("Console::doGuestPropNotification: pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n", - pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags)); + LogFlow(("Console::doGuestPropNotification: pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n", + pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags)); int rc; @@ -1714,5 +1714,5 @@ else { - LogFunc(("Console::doGuestPropNotification: hrc=%Rhrc pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n", + LogFlow(("Console::doGuestPropNotification: hrc=%Rhrc pCBData={.pcszName=%s, .pcszValue=%s, .pcszFlags=%s}\n", hrc, pCBData->pcszName, pCBData->pcszValue, pCBData->pcszFlags)); rc = Global::vboxStatusCodeFromCOM(hrc); Index: /trunk/src/VBox/Main/src-server/MachineImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/MachineImpl.cpp (revision 44166) +++ /trunk/src/VBox/Main/src-server/MachineImpl.cpp (revision 44167) @@ -5444,19 +5444,16 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); Utf8Str strName(aName); - HWData::GuestPropertyList::const_iterator it; - - for (it = mHWData->mGuestProperties.begin(); - it != mHWData->mGuestProperties.end(); ++it) - { - if (it->strName == strName) - { - char szFlags[MAX_FLAGS_LEN + 1]; - it->strValue.cloneTo(aValue); - *aTimestamp = it->mTimestamp; - writeFlags(it->mFlags, szFlags); - Bstr(szFlags).cloneTo(aFlags); - break; - } - } + HWData::GuestPropertyMap::const_iterator it = + mHWData->mGuestProperties.find(strName); + + if (it != mHWData->mGuestProperties.end()) + { + char szFlags[MAX_FLAGS_LEN + 1]; + it->second.strValue.cloneTo(aValue); + *aTimestamp = it->second.mTimestamp; + writeFlags(it->second.mFlags, szFlags); + Bstr(szFlags).cloneTo(aFlags); + } + return S_OK; } @@ -5541,5 +5538,4 @@ HWData::GuestProperty property; property.mFlags = NILFLAG; - bool found = false; rc = checkStateDependency(MutableStateDep); @@ -5551,62 +5547,60 @@ Utf8Str utf8Flags(aFlags); uint32_t fFlags = NILFLAG; - if ( (aFlags != NULL) - && RT_FAILURE(validateFlags(utf8Flags.c_str(), &fFlags)) + if ( (aFlags != NULL) + && RT_FAILURE(validateFlags(utf8Flags.c_str(), &fFlags)) ) return setError(E_INVALIDARG, - tr("Invalid flag values: '%ls'"), + tr("Invalid guest property flag values: '%ls'"), aFlags); - /** @todo r=bird: see efficiency rant in PushGuestProperty. (Yeah, I - * know, this is simple and do an OK job atm.) */ - HWData::GuestPropertyList::iterator it; - for (it = mHWData->mGuestProperties.begin(); - it != mHWData->mGuestProperties.end(); ++it) - if (it->strName == utf8Name) + HWData::GuestPropertyMap::iterator it = + mHWData->mGuestProperties.find(utf8Name); + + if (it == mHWData->mGuestProperties.end()) + { + setModified(IsModified_MachineData); + mHWData.backupEx(); + + RTTIMESPEC time; + HWData::GuestProperty prop; + prop.strValue = aValue; + prop.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time)); + prop.mFlags = fFlags; + + mHWData->mGuestProperties[Utf8Str(aName)] = prop; + } + else + { + if (it->second.mFlags & (RDONLYHOST)) { - property = *it; - if (it->mFlags & (RDONLYHOST)) - rc = setError(E_ACCESSDENIED, - tr("The property '%ls' cannot be changed by the host"), - aName); - else - { - setModified(IsModified_MachineData); - mHWData.backup(); // @todo r=dj backup in a loop?!? - - /* The backup() operation invalidates our iterator, so - * get a new one. */ - for (it = mHWData->mGuestProperties.begin(); - it->strName != utf8Name; - ++it) - ; - mHWData->mGuestProperties.erase(it); - } - found = true; - break; + rc = setError(E_ACCESSDENIED, + tr("The property '%ls' cannot be changed by the host"), + aName); } - if (found && SUCCEEDED(rc)) - { - if (aValue) + else if (aValue) { + setModified(IsModified_MachineData); + mHWData.backupEx(); + + /* The backupEx() operation invalidates our iterator, + * so get a new one. */ + it = mHWData->mGuestProperties.find(utf8Name); + Assert(it != mHWData->mGuestProperties.end()); + RTTIMESPEC time; - property.strValue = aValue; - property.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time)); + it->second.strValue = aValue; + it->second.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time)); if (aFlags != NULL) - property.mFlags = fFlags; - mHWData->mGuestProperties.push_back(property); + it->second.mFlags = fFlags; } - } - else if (SUCCEEDED(rc) && aValue) - { - RTTIMESPEC time; - setModified(IsModified_MachineData); - mHWData.backup(); - property.strName = aName; - property.strValue = aValue; - property.mTimestamp = RTTimeSpecGetNano(RTTimeNow(&time)); - property.mFlags = fFlags; - mHWData->mGuestProperties.push_back(property); - } + else + { + setModified(IsModified_MachineData); + mHWData.backupEx(); + + mHWData->mGuestProperties.erase(it); + } + } + if ( SUCCEEDED(rc) && ( mHWData->mGuestPropertyNotificationPatterns.isEmpty() @@ -5619,6 +5613,6 @@ ) { - /** @todo r=bird: Why aren't we leaving the lock here? The - * same code in PushGuestProperty does... */ + alock.release(); + mParent->onGuestPropertyChange(mData->mUuid, aName, aValue ? aValue : Bstr("").raw(), @@ -5716,24 +5710,32 @@ Utf8Str strPatterns(aPatterns); + HWData::GuestPropertyMap propMap; + /* * Look for matching patterns and build up a list. */ - HWData::GuestPropertyList propList; - for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin(); - it != mHWData->mGuestProperties.end(); - ++it) + HWData::GuestPropertyMap::const_iterator it = mHWData->mGuestProperties.begin(); + while (it != mHWData->mGuestProperties.end()) + { if ( strPatterns.isEmpty() || RTStrSimplePatternMultiMatch(strPatterns.c_str(), RTSTR_MAX, - it->strName.c_str(), + it->first.c_str(), RTSTR_MAX, NULL) ) - propList.push_back(*it); + { + propMap.insert(*it); + } + + it++; + } + + alock.release(); /* * And build up the arrays for returning the property information. */ - size_t cEntries = propList.size(); + size_t cEntries = propMap.size(); SafeArray names(cEntries); SafeArray values(cEntries); @@ -5741,15 +5743,15 @@ SafeArray flags(cEntries); size_t iProp = 0; - for (HWData::GuestPropertyList::iterator it = propList.begin(); - it != propList.end(); - ++it) + + it = propMap.begin(); + while (it != propMap.end()) { char szFlags[MAX_FLAGS_LEN + 1]; - it->strName.cloneTo(&names[iProp]); - it->strValue.cloneTo(&values[iProp]); - timestamps[iProp] = it->mTimestamp; - writeFlags(it->mFlags, szFlags); - Bstr(szFlags).cloneTo(&flags[iProp]); - ++iProp; + it->first.cloneTo(&names[iProp]); + it->second.strValue.cloneTo(&values[iProp]); + timestamps[iProp] = it->second.mTimestamp; + writeFlags(it->second.mFlags, szFlags); + Bstr(szFlags).cloneTo(&flags[iProp++]); + it++; } names.detachTo(ComSafeArrayOutArg(aNames)); @@ -8674,6 +8676,6 @@ uint32_t fFlags = guestProp::NILFLAG; guestProp::validateFlags(prop.strFlags.c_str(), &fFlags); - HWData::GuestProperty property = { prop.strName, prop.strValue, (LONG64) prop.timestamp, fFlags }; - mHWData->mGuestProperties.push_back(property); + HWData::GuestProperty property = { prop.strValue, (LONG64) prop.timestamp, fFlags }; + mHWData->mGuestProperties[prop.strName] = property; } @@ -9857,9 +9859,9 @@ data.llGuestProperties.clear(); #ifdef VBOX_WITH_GUEST_PROPS - for (HWData::GuestPropertyList::const_iterator it = mHWData->mGuestProperties.begin(); + for (HWData::GuestPropertyMap::const_iterator it = mHWData->mGuestProperties.begin(); it != mHWData->mGuestProperties.end(); ++it) { - HWData::GuestProperty property = *it; + HWData::GuestProperty property = it->second; /* Remove transient guest properties at shutdown unless we @@ -9872,5 +9874,5 @@ continue; settings::GuestProperty prop; - prop.strName = property.strName; + prop.strName = it->first; prop.strValue = property.strValue; prop.timestamp = property.mTimestamp; @@ -12745,16 +12747,16 @@ com::SafeArray flags(cEntries); unsigned i = 0; - for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin(); + for (HWData::GuestPropertyMap::iterator it = mHWData->mGuestProperties.begin(); it != mHWData->mGuestProperties.end(); ++it) { char szFlags[MAX_FLAGS_LEN + 1]; - it->strName.cloneTo(&names[i]); - it->strValue.cloneTo(&values[i]); - timestamps[i] = it->mTimestamp; + it->first.cloneTo(&names[i]); + it->second.strValue.cloneTo(&values[i]); + timestamps[i] = it->second.mTimestamp; /* If it is NULL, keep it NULL. */ - if (it->mFlags) - { - writeFlags(it->mFlags, szFlags); + if (it->second.mFlags) + { + writeFlags(it->second.mFlags, szFlags); Bstr(szFlags).cloneTo(&flags[i]); } @@ -12833,27 +12835,16 @@ mHWData.backup(); - /** @todo r=bird: The careful memory handling doesn't work out here because - * the catch block won't undo any damage we've done. So, if push_back throws - * bad_alloc then you've lost the value. - * - * Another thing. Doing a linear search here isn't extremely efficient, esp. - * since values that changes actually bubbles to the end of the list. Using - * something that has an efficient lookup and can tolerate a bit of updates - * would be nice. RTStrSpace is one suggestion (it's not perfect). Some - * combination of RTStrCache (for sharing names and getting uniqueness into - * the bargain) and hash/tree is another. */ - for (HWData::GuestPropertyList::iterator iter = mHWData->mGuestProperties.begin(); - iter != mHWData->mGuestProperties.end(); - ++iter) - if (utf8Name == iter->strName) + HWData::GuestPropertyMap::iterator it = mHWData->mGuestProperties.find(utf8Name); + if (it != mHWData->mGuestProperties.end()) + { + if (aValue != NULL) { - mHWData->mGuestProperties.erase(iter); - mData->mGuestPropertiesModified = TRUE; - break; + it->second.strValue = aValue; + it->second.mFlags = fFlags; + it->second.mTimestamp = aTimestamp; } - if (aValue != NULL) - { - HWData::GuestProperty property = { aName, aValue, aTimestamp, fFlags }; - mHWData->mGuestProperties.push_back(property); + else + mHWData->mGuestProperties.erase(it); + mData->mGuestPropertiesModified = TRUE; } @@ -13921,11 +13912,11 @@ * property store on shutdown. */ - HWData::GuestPropertyList::iterator it; + HWData::GuestPropertyMap::const_iterator it; BOOL fNeedsSaving = mData->mGuestPropertiesModified; if (!fNeedsSaving) for (it = mHWData->mGuestProperties.begin(); it != mHWData->mGuestProperties.end(); ++it) - if ( (it->mFlags & guestProp::TRANSIENT) - || (it->mFlags & guestProp::TRANSRESET)) + if ( (it->second.mFlags & guestProp::TRANSIENT) + || (it->second.mFlags & guestProp::TRANSRESET)) { fNeedsSaving = true;