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

Timestamp:

Aug 12, 2020 8:26:52 PM (4 years ago)

Author:

vboxsync

Message:

Main/HostUpdateImpl.cpp: updateCheck() must validate the bugref:7983updateCheck

File:

• trunk/src/VBox/Main/src-server/HostUpdateImpl.cpp (modified) (15 diffs)

trunk/src/VBox/Main/src-server/HostUpdateImpl.cpp

@@ -74 +74 @@
     /** Smart pointer to the progress object for this job. */
     ComObjPtr<Progress> m_ptrProgress;
-    HRESULT m_rc; /**< Not really used for anything, at the moment. */
+    HRESULT m_rc; /**< Not really used for anything outside i_updateCheckTask, at the moment. */
 
     friend class HostUpdate;  // allow member functions access to private data
@@ -403 +403 @@
     else
     {
+        /** @todo r=bird: trusting the server reply too much here! */
         m_updateNeeded = TRUE;
         m_updateVersion = lstHttpReply.at(0).c_str();
@@ -419 +420 @@
 {
     LogFlowFuncEnter();
-
     AutoCaller autoCaller(this);
-    if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-    if (pTask->m_checkType == UpdateCheckType_VirtualBox)
-        pTask->m_rc = i_checkForVBoxUpdate();
-
+    HRESULT hrc = autoCaller.rc();
+    if (SUCCEEDED(hrc))
+    {
+        try
+        {
+            switch (pTask->m_checkType)
+            {
+                case UpdateCheckType_VirtualBox:
+                    hrc = i_checkForVBoxUpdate();
+                    break;
 #if 0
-    else if (pTask->m_checkType == UpdateCheckType_ExtensionPack)
-        ;
-    else if (pTask->m_checkType == UpdateCheckType_GuestAdditions)
-        ;
-    else
-        assert();
+                case UpdateCheckType_ExtensionPack:
+                    hrc = i_checkForExtPackUpdate();
+                    break;
+
+                case UpdateCheckType_GuestAdditions:
+                    hrc = i_checkForGuestAdditionsUpdate();
+                    break;
 #endif
+                default:
+                    hrc = setError(E_FAIL, tr("Update check type %d is not implemented"), pTask->m_checkType);
+                    break;
+            }
+        }
+        catch (...)
+        {
+            AssertFailed();
+            hrc = E_UNEXPECTED;
+        }
+    }
 
     if (!pTask->m_ptrProgress.isNull())
-        pTask->m_ptrProgress->i_notifyComplete(pTask->m_rc);
-
-    LogFlowFunc(("rc=%Rhrc\n", pTask->m_rc));
+        pTask->m_ptrProgress->i_notifyComplete(hrc);
+
+    LogFlowFunc(("rc=%Rhrc\n", hrc));
     LogFlowFuncLeave();
-
-    return pTask->m_rc;
+    return pTask->m_rc = hrc;
 }
 
@@ -495 +511 @@
                                 ComPtr<IProgress> &aProgress)
 {
-    HRESULT rc;
+    /* Validate input */
+    switch (aCheckType)
+    {
+        case UpdateCheckType_VirtualBox:
+            break;
+        case UpdateCheckType_ExtensionPack:
+            return setError(E_NOTIMPL, tr("UpdateCheckType::ExtensionPack is not implemented"));
+        case UpdateCheckType_GuestAdditions:
+            return setError(E_NOTIMPL, tr("UpdateCheckType::GuestAdditions is not implemented"));
+        default:
+            return setError(E_INVALIDARG, tr("Invalid aCheckType value %d"), aCheckType);
+    }
 
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
@@ -501 +528 @@
     // Check whether VirtualBox updates have been disabled before spawning the task thread.
     ComPtr<ISystemProperties> pSystemProperties;
-    rc = mVirtualBox->COMGETTER(SystemProperties)(pSystemProperties.asOutParam());
+    HRESULT rc = mVirtualBox->COMGETTER(SystemProperties)(pSystemProperties.asOutParam());
     if (FAILED(rc))
         return setErrorVrc(rc, tr("%s: IVirtualBox::systemProperties() failed: %Rrc"), __FUNCTION__, rc);
@@ -510 +537 @@
         return setErrorVrc(rc, tr("%s: retrieving ISystemProperties::VBoxUpdateEnabled failed: %Rrc"), __FUNCTION__, rc);
 
+    /** @todo r=bird: Not sure if this makes sense, it should at least have a
+     * better status code and a proper error message.  Also, isn't this really
+     * something the caller should check?  Presumably the caller already check
+     * whther this was a good time to perform an update check (i.e. the configured
+     * time has elapsed since last check) ...
+     *
+     * It would make sense to allow performing a one-off update check even if the
+     * automatic update checking is disabled, wouldn't it? */
     if (!fVBoxUpdateEnabled)
         return E_NOTIMPL;
@@ -516 +551 @@
     rc = pProgress.createObject();
     if (FAILED(rc))
-    {
         return rc;
-    }
 
     rc = pProgress->init(mVirtualBox,
@@ -525 +558 @@
                          TRUE /* aCancelable */);
     if (FAILED(rc))
-    {
         return rc;
-    }
 
     /* initialize the worker task */
@@ -534 +565 @@
     pTask = NULL;
     if (FAILED(rc))
-    {
         return rc;
-    }
 
     rc = pProgress.queryInterfaceTo(aProgress.asOutParam());
@@ -547 +576 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    if (m_updateVersion.isNotEmpty())
-    {
-        aUpdateVersion = m_updateVersion;
-    }
+    aUpdateVersion = m_updateVersion;
 
     return S_OK;
@@ -559 +585 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    if (m_updateURL.isNotEmpty())
-    {
-        aUpdateURL = m_updateURL;
-    }
+    aUpdateURL = m_updateURL;
 
     return S_OK;
@@ -586 +609 @@
         return rc;
 
+    /*
+     * Is update checking enabled?
+     */
     BOOL fVBoxUpdateEnabled;
     rc = pSystemProperties->COMGETTER(VBoxUpdateEnabled)(&fVBoxUpdateEnabled);
@@ -597 +623 @@
     }
 
+    /*
+     * When was the last update?
+     */
     Bstr strVBoxUpdateLastCheckDate;
     rc = pSystemProperties->COMGETTER(VBoxUpdateLastCheckDate)(strVBoxUpdateLastCheckDate.asOutParam());
@@ -617 +646 @@
     }
 
+    /*
+     * Compare last update with how often we are supposed to check for updates.
+     */
     ULONG uVBoxUpdateFrequency = 0;  // value in days
     rc = pSystemProperties->COMGETTER(VBoxUpdateFrequency)(&uVBoxUpdateFrequency);
@@ -624 +656 @@
     if (!uVBoxUpdateFrequency)
     {
+        /* Consider config (enable, 0 day interval) as checking once but never again.
+           We've already check since we've got a date. */
         *aUpdateCheckNeeded = false;
         return S_OK;
     }
-
-    ULONG ulSecondsInXDays = uVBoxUpdateFrequency /* in days */ * 24 /* hours */ * 60 /* minutes */ * 60 /* seconds */;
-    RTTIMESPEC TimeNow;
-    (void) RTTimeNow(&TimeNow);
-    PRTTIMESPEC TimeDiff = RTTimeSpecSub(&TimeNow, &LastCheckTime);
-
-    LogRelFunc(("Checking if seconds since last check (%ld) >= Number of seconds in %lu day%s (%ld)\n",
-        RTTimeSpecGetSeconds(TimeDiff), uVBoxUpdateFrequency, uVBoxUpdateFrequency > 1 ? "s" : "", ulSecondsInXDays));
-
-    if (RTTimeSpecGetSeconds(TimeDiff) >= ulSecondsInXDays)
+    uint64_t const cSecsInXDays = uVBoxUpdateFrequency * RT_SEC_1DAY_64;
+
+    RTTIMESPEC TimeDiff;
+    RTTimeSpecSub(RTTimeNow(&TimeDiff), &LastCheckTime);
+
+    LogRelFunc(("Checking if seconds since last check (%lld) >= Number of seconds in %lu day%s (%lld)\n",
+                RTTimeSpecGetSeconds(&TimeDiff), uVBoxUpdateFrequency, uVBoxUpdateFrequency > 1 ? "s" : "", cSecsInXDays));
+
+    if (RTTimeSpecGetSeconds(&TimeDiff) >= (int64_t)cSecsInXDays)
         *aUpdateCheckNeeded = true;