VirtualBox

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


Ignore:
Timestamp:
Nov 14, 2009 11:55:15 PM (15 years ago)
Author:
vboxsync
Message:

MachineImpl.cpp: Some reviewing / fussing / optimizing of the guest property code. Changed mGuestPropertyNotificationPatterns from Bstr to Utf8Str to avoid coverting it every time it is used.

Location:
trunk/src/VBox/Main
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/VBox/Main/MachineImpl.cpp

    r24649 r24663  
    20292029
    20302030STDMETHODIMP
    2031 Machine::COMGETTER(GuestPropertyNotificationPatterns) (BSTR *aPatterns)
     2031Machine::COMGETTER(GuestPropertyNotificationPatterns)(BSTR *aPatterns)
    20322032{
    20332033    CheckComArgOutPointerValid(aPatterns);
     
    20382038    AutoReadLock alock(this);
    20392039
    2040     mHWData->mGuestPropertyNotificationPatterns.cloneTo(aPatterns);
    2041 
    2042     return RT_LIKELY (aPatterns != NULL) ? S_OK : E_OUTOFMEMORY; /** @todo r=bird: this is wrong... :-) */
     2040    try
     2041    {
     2042        mHWData->mGuestPropertyNotificationPatterns.cloneTo(aPatterns);
     2043    }
     2044    catch (...)
     2045    {
     2046        return VirtualBox::handleUnexpectedExceptions(RT_SRC_POS);
     2047    }
     2048
     2049    return S_OK;
    20432050}
    20442051
    20452052STDMETHODIMP
    2046 Machine::COMSETTER(GuestPropertyNotificationPatterns) (IN_BSTR aPatterns)
    2047 {
    2048     AssertLogRelReturn (VALID_PTR (aPatterns), E_POINTER);
     2053Machine::COMSETTER(GuestPropertyNotificationPatterns)(IN_BSTR aPatterns)
     2054{
     2055    CheckComArgNotNull(aPatterns);
    20492056    AutoCaller autoCaller(this);
    20502057    CheckComRCReturnRC(autoCaller.rc());
     
    20552062    CheckComRCReturnRC(rc);
    20562063
    2057     mHWData.backup();
    2058     mHWData->mGuestPropertyNotificationPatterns = aPatterns;
    2059 
    2060     return RT_LIKELY (!mHWData->mGuestPropertyNotificationPatterns.isNull())
    2061                ? S_OK : E_OUTOFMEMORY;
     2064    try
     2065    {
     2066        mHWData.backup();
     2067        mHWData->mGuestPropertyNotificationPatterns = aPatterns;
     2068    }
     2069    catch (...)
     2070    {
     2071        rc = VirtualBox::handleUnexpectedExceptions(RT_SRC_POS);
     2072    }
     2073    return rc;
    20622074}
    20632075
     
    35183530    try
    35193531    {
     3532        Utf8Str utf8Name(aName);
     3533        Utf8Str utf8Flags(aFlags);
     3534
    35203535        AutoCaller autoCaller(this);
    35213536        CheckComRCReturnRC(autoCaller.rc());
     
    35273542
    35283543        rc = S_OK;
    3529 
    3530         Utf8Str utf8Name(aName);
    3531         Utf8Str utf8Flags(aFlags);
    3532         Utf8Str utf8Patterns(mHWData->mGuestPropertyNotificationPatterns);
    3533 
    3534         bool matchAll = false;
    3535         if (utf8Patterns.isEmpty())
    3536             matchAll = true;
    3537 
    35383544        uint32_t fFlags = NILFLAG;
    35393545        if (    (aFlags != NULL)
     
    35503556            property.mFlags = NILFLAG;
    35513557
    3552             if (SUCCEEDED(rc))
    3553             {
    3554                 for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin();
    3555                      it != mHWData->mGuestProperties.end();
    3556                      ++it)
    3557                     if (it->strName == utf8Name)
     3558            /** @todo r=bird: see efficiency rant in PushGuestProperty. (Yeah, I know,
     3559             *        this is simple and do an OK job atm.) */
     3560            for (HWData::GuestPropertyList::iterator it = mHWData->mGuestProperties.begin();
     3561                 it != mHWData->mGuestProperties.end();
     3562                 ++it)
     3563                if (it->strName == utf8Name)
     3564                {
     3565                    property = *it;
     3566                    if (it->mFlags & (RDONLYHOST))
     3567                        rc = setError(E_ACCESSDENIED,
     3568                                      tr("The property '%ls' cannot be changed by the host"),
     3569                                      aName);
     3570                    else
    35583571                    {
    3559                         property = *it;
    3560                         if (it->mFlags & (RDONLYHOST))
    3561                             rc = setError(E_ACCESSDENIED,
    3562                                           tr("The property '%ls' cannot be changed by the host"),
    3563                                           aName);
    3564                         else
    3565                         {
    3566                             mHWData.backup();
    3567                             /* The backup() operation invalidates our iterator, so
    3568                             * get a new one. */
    3569                             for (it = mHWData->mGuestProperties.begin();
    3570                                  it->strName != utf8Name;
    3571                                  ++it)
    3572                                 ;
    3573                             mHWData->mGuestProperties.erase (it);
    3574                         }
    3575                         found = true;
    3576                         break;
     3572                        mHWData.backup();
     3573                        /* The backup() operation invalidates our iterator, so
     3574                        * get a new one. */
     3575                        for (it = mHWData->mGuestProperties.begin();
     3576                             it->strName != utf8Name;
     3577                             ++it)
     3578                            ;
     3579                        mHWData->mGuestProperties.erase (it);
    35773580                    }
    3578             }
     3581                    found = true;
     3582                    break;
     3583                }
    35793584            if (found && SUCCEEDED(rc))
    35803585            {
     
    36003605            }
    36013606            if (   SUCCEEDED(rc)
    3602                 && (   matchAll
    3603                     || RTStrSimplePatternMultiMatch (utf8Patterns.raw(), RTSTR_MAX,
    3604                                                     utf8Name.raw(), RTSTR_MAX, NULL)
    3605                 )
    3606             )
    3607                 mParent->onGuestPropertyChange (mData->mUuid, aName, aValue, aFlags);
     3607                && (   mHWData->mGuestPropertyNotificationPatterns.isEmpty()
     3608                    || RTStrSimplePatternMultiMatch(mHWData->mGuestPropertyNotificationPatterns.raw(), RTSTR_MAX,
     3609                                                    utf8Name.raw(), RTSTR_MAX, NULL) )
     3610               )
     3611            {
     3612                /** @todo r=bird: Why aren't we leaving the lock here?  The
     3613                 *                same code in PushGuestProperty does... */
     3614                mParent->onGuestPropertyChange(mData->mUuid, aName, aValue, aFlags);
     3615            }
    36083616        }
    36093617        else
     
    36203628                rc = E_FAIL;
    36213629            else
    3622                 rc = directControl->AccessGuestProperty
    3623                              (aName, *aValue ? aValue : NULL, aFlags,
    3624                               true /* isSetter */, &dummy, &dummy64, &dummy);
     3630                rc = directControl->AccessGuestProperty(aName,
     3631                                                        *aValue ? aValue : NULL,  /** @todo Fix when adding DeleteGuestProperty(), see defect. */
     3632                                                        aFlags,
     3633                                                        true /* isSetter */,
     3634                                                        &dummy, &dummy64, &dummy);
    36253635        }
    36263636    }
     
    36663676    Utf8Str strPatterns(aPatterns);
    36673677
    3668     bool matchAll = false;
    3669     if ((NULL == aPatterns) || (0 == aPatterns[0]))
    3670         matchAll = true;
    36713678    if (!mHWData->mPropertyServiceActive)
    36723679    {
     
    36793686             it != mHWData->mGuestProperties.end();
    36803687             ++it)
    3681             if (   matchAll
     3688            if (   strPatterns.isEmpty()
    36823689                || RTStrSimplePatternMultiMatch(strPatterns.raw(),
    36833690                                                RTSTR_MAX,
     
    92089215}
    92099216
    9210 STDMETHODIMP SessionMachine::PushGuestProperties(ComSafeArrayIn (IN_BSTR, aNames),
    9211                                                  ComSafeArrayIn (IN_BSTR, aValues),
    9212                                                  ComSafeArrayIn (ULONG64, aTimestamps),
    9213                                                  ComSafeArrayIn (IN_BSTR, aFlags))
     9217STDMETHODIMP SessionMachine::PushGuestProperties(ComSafeArrayIn(IN_BSTR, aNames),
     9218                                                 ComSafeArrayIn(IN_BSTR, aValues),
     9219                                                 ComSafeArrayIn(ULONG64, aTimestamps),
     9220                                                 ComSafeArrayIn(IN_BSTR, aFlags))
    92149221{
    92159222    LogFlowThisFunc(("\n"));
     
    92189225    using namespace guestProp;
    92199226
    9220     AutoCaller autoCaller(this);
    9221     AssertComRCReturn (autoCaller.rc(), autoCaller.rc());
     9227    AssertReturn(!ComSafeArrayInIsNull(aNames),      E_POINTER);
     9228    AssertReturn(!ComSafeArrayInIsNull(aValues),     E_POINTER);
     9229    AssertReturn(!ComSafeArrayInIsNull(aTimestamps), E_POINTER);
     9230    AssertReturn(!ComSafeArrayInIsNull(aFlags),      E_POINTER);
     9231
     9232    AutoCaller autoCaller(this);
     9233    AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
    92229234
    92239235    AutoWriteLock alock(this);
    92249236
    9225     /* Temporarily reset the registered flag, so that our machine state
    9226      * changes (i.e. mHWData.backup()) succeed.  (isMutable() used in
    9227      * all setters will return FALSE for a Machine instance if mRegistered
    9228      * is TRUE).  This is copied from registeredInit(), and may or may not be
    9229      * the right way to handle this. */
     9237    /*
     9238     * Temporarily reset the registered flag, so that our machine state
     9239     * changes (i.e. mHWData.backup()) succeed.  (isMutable() used in all
     9240     * setters will return FALSE for a Machine instance if mRegistered is TRUE).
     9241     *
     9242     * This is copied from registeredInit(), and may or may not be the right
     9243     * way to handle this.
     9244     */
     9245    Assert(!mData->mRegistered);
    92309246    mData->mRegistered = FALSE;
     9247
    92319248    HRESULT rc = checkStateDependency(MutableStateDep);
    9232     LogRel (("checkStateDependency(MutableStateDep) returned 0x%x\n", rc));
    9233     CheckComRCReturnRC(rc);
    9234 
    9235     // ComAssertRet (mData->mMachineState < MachineState_Running, E_FAIL);
    9236 
    9237     AssertReturn(!ComSafeArrayInIsNull (aNames), E_POINTER);
    9238     AssertReturn(!ComSafeArrayInIsNull (aValues), E_POINTER);
    9239     AssertReturn(!ComSafeArrayInIsNull (aTimestamps), E_POINTER);
    9240     AssertReturn(!ComSafeArrayInIsNull (aFlags), E_POINTER);
    9241 
    9242     com::SafeArray<IN_BSTR> names (ComSafeArrayInArg (aNames));
    9243     com::SafeArray<IN_BSTR> values (ComSafeArrayInArg (aValues));
    9244     com::SafeArray<ULONG64> timestamps (ComSafeArrayInArg (aTimestamps));
    9245     com::SafeArray<IN_BSTR> flags (ComSafeArrayInArg (aFlags));
     9249    AssertLogRelMsgReturn(SUCCEEDED(rc), ("%Rhrc\n", rc), rc);
     9250
     9251    com::SafeArray<IN_BSTR> names(     ComSafeArrayInArg(aNames));
     9252    com::SafeArray<IN_BSTR> values(    ComSafeArrayInArg(aValues));
     9253    com::SafeArray<ULONG64> timestamps(ComSafeArrayInArg(aTimestamps));
     9254    com::SafeArray<IN_BSTR> flags(     ComSafeArrayInArg(aFlags));
     9255
    92469256    DiscardSettings();
    92479257    mHWData.backup();
    9248     mHWData->mGuestProperties.erase (mHWData->mGuestProperties.begin(),
     9258
     9259    mHWData->mGuestProperties.erase(mHWData->mGuestProperties.begin(),
    92499260                                    mHWData->mGuestProperties.end());
    92509261    for (unsigned i = 0; i < names.size(); ++i)
    92519262    {
    92529263        uint32_t fFlags = NILFLAG;
    9253         validateFlags (Utf8Str (flags[i]).raw(), &fFlags);
     9264        validateFlags(Utf8Str(flags[i]).raw(), &fFlags);
    92549265        HWData::GuestProperty property = { names[i], values[i], timestamps[i], fFlags };
    9255         mHWData->mGuestProperties.push_back (property);
    9256     }
     9266        mHWData->mGuestProperties.push_back(property);
     9267    }
     9268
    92579269    mHWData->mPropertyServiceActive = false;
     9270
    92589271    alock.unlock();
    92599272    SaveSettings();
     9273
    92609274    /* Restore the mRegistered flag. */
     9275    alock.lock();
    92619276    mData->mRegistered = TRUE;
     9277
    92629278    return S_OK;
    92639279#else
     
    92769292    using namespace guestProp;
    92779293
    9278     CheckComArgNotNull (aName);
    9279     if ((aValue != NULL) && (!VALID_PTR (aValue) || !VALID_PTR (aFlags)))
     9294    CheckComArgNotNull(aName);
     9295    if (aValue != NULL && (!VALID_PTR(aValue) || !VALID_PTR(aFlags)))
    92809296        return E_POINTER;  /* aValue can be NULL to indicate deletion */
    92819297
    92829298    try
    92839299    {
     9300        /*
     9301         * Convert input up front.
     9302         */
     9303        Utf8Str     utf8Name(aName);
     9304        uint32_t    fFlags = NILFLAG;
     9305        if (aFlags)
     9306        {
     9307            Utf8Str utf8Flags(aFlags);
     9308            int vrc = validateFlags(utf8Flags.raw(), &fFlags);
     9309            AssertRCReturn(vrc, E_INVALIDARG);
     9310        }
     9311
     9312        /*
     9313         * Now grab the object lock and do the update
     9314         */
    92849315        AutoCaller autoCaller(this);
    92859316        CheckComRCReturnRC(autoCaller.rc());
     
    92879318        AutoWriteLock alock(this);
    92889319
     9320        AssertReturn(mHWData->mPropertyServiceActive, VBOX_E_INVALID_OBJECT_STATE);
     9321
    92899322        HRESULT rc = checkStateDependency(MutableStateDep);
    92909323        CheckComRCReturnRC(rc);
    92919324
    9292         Utf8Str utf8Name(aName);
    9293         Utf8Str utf8Flags(aFlags);
    9294         Utf8Str utf8Patterns(mHWData->mGuestPropertyNotificationPatterns);
    9295 
    9296         uint32_t fFlags = NILFLAG;
    9297         if ((aFlags != NULL) && RT_FAILURE(validateFlags (utf8Flags.raw(), &fFlags)))
    9298             return E_INVALIDARG;
    9299 
    9300         bool matchAll = false;
    9301         if (utf8Patterns.isEmpty())
    9302             matchAll = true;
    9303 
    93049325        mHWData.backup();
     9326
     9327        /** @todo r=bird: The careful memory handling doesn't work out here because
     9328         *  the catch block won't undo any damange we've done.  So, if push_back throws
     9329         *  bad_alloc then you've lost the value.
     9330         *
     9331         *  Another thing. Doing a linear search here isn't extremely efficient, esp.
     9332         *  since values that changes actually bubbles to the end of the list.  Using
     9333         *  something that has an efficient lookup and can tollerate a bit of updates
     9334         *  would be nice.  RTStrSpace is one suggestion (it's not perfect).  Some
     9335         *  combination of RTStrCache (for sharing names and getting uniqueness into
     9336         *  the bargain) and hash/tree is another. */
    93059337        for (HWData::GuestPropertyList::iterator iter = mHWData->mGuestProperties.begin();
    93069338             iter != mHWData->mGuestProperties.end();
     
    93149346        {
    93159347            HWData::GuestProperty property = { aName, aValue, aTimestamp, fFlags };
    9316             mHWData->mGuestProperties.push_back (property);
    9317         }
    9318 
    9319         /* send a callback notification if appropriate */
    9320         alock.leave();
    9321         if (    matchAll
    9322              || RTStrSimplePatternMultiMatch(utf8Patterns.raw(),
     9348            mHWData->mGuestProperties.push_back(property);
     9349        }
     9350
     9351        /*
     9352         * Send a callback notification if appropriate
     9353         */
     9354        if (    mHWData->mGuestPropertyNotificationPatterns.isEmpty()
     9355             || RTStrSimplePatternMultiMatch(mHWData->mGuestPropertyNotificationPatterns.raw(),
    93239356                                             RTSTR_MAX,
    93249357                                             utf8Name.raw(),
     
    93269359           )
    93279360        {
     9361            alock.leave();
     9362
    93289363            mParent->onGuestPropertyChange(mData->mUuid,
    93299364                                           aName,
     
    93329367        }
    93339368    }
    9334     catch(std::bad_alloc &)
    9335     {
    9336         return E_OUTOFMEMORY;
     9369    catch (...)
     9370    {
     9371        return VirtualBox::handleUnexpectedExceptions(RT_SRC_POS);
    93379372    }
    93389373    return S_OK;
  • trunk/src/VBox/Main/include/MachineImpl.h

    r24558 r24663  
    312312        GuestPropertyList mGuestProperties;
    313313        BOOL           mPropertyServiceActive;
    314         Bstr           mGuestPropertyNotificationPatterns;
     314        Utf8Str        mGuestPropertyNotificationPatterns;
    315315
    316316        FirmwareType_T mFirmwareType;
Note: See TracChangeset for help on using the changeset viewer.

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette