Changeset 84535 in vbox for trunk/src

Timestamp:

May 26, 2020 11:05:07 AM (5 years ago)

Author:

vboxsync

Message:

Main/Appliance::i_importCloudImpl: %ul is wrong. Just use %RU64 and combine the two appends into a single call. Check status code. If you want to restore a previous hrc value, you must also restore the associated error info or we'll have some really confusing error messages. The first hrc restoring after cleanups looks wrong, as it's ignoring cleanup errors. bugref:9416

File:

• trunk/src/VBox/Main/src-server/ApplianceImplImport.cpp (modified) (5 diffs)

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

@@ -1454 +1454 @@
             {
                 /* what to do? create a new name from the old one with some suffix? */
-                uint64_t suff = RTRandU64();
-                strVMName.append("__").appendPrintfNoThrow("%ul", suff);
+                uint64_t uRndSuff = RTRandU64();
+                vrc = strVMName.appendPrintfNoThrow("__%RU64", uRndSuff);
+                AssertRCBreakStmt(vrc, hrc = E_OUTOFMEMORY);
 
                 vsd->RemoveDescriptionByType(VirtualSystemDescriptionType_Name);
@@ -1546 +1547 @@
 
 
-    HRESULT original_hrc = hrc;//save the original result
-
     /* In any case we delete the cloud leavings which may exist after the first phase (cloud phase).
      * Should they be deleted in the OCICloudClient::importInstance()?
      * Because deleting them here is not easy as it in the importInstance(). */
     {
+        ErrorInfoKeeper eik;    /* save the error info */
+        HRESULT const hrcSaved = hrc;
+
         GET_VSD_DESCRIPTION_BY_TYPE(VirtualSystemDescriptionType_CloudInstanceId)//aVBoxValues is set in this #define
         if (aVBoxValues.size() == 0)
@@ -1580 +1582 @@
          * Thus we restore the original error in the case when the cleanup phase was successful
          * Otherwise we return not the original error but the last error in the cleanup phase */
-         hrc = original_hrc;
+        /** @todo r=bird: do this conditionally perhaps?
+         * if (FAILED(hrcSaved))
+         *     hrc = hrcSaved;
+         * else
+         *     eik.forget();
+         */
+        hrc = hrcSaved;
     }
 
     if (FAILED(hrc))
     {
+        /** @todo r=bird: Using heap to keep a readonly C-string is a real wonderful
+         *        way to doing things. */
         Utf8Str generalRollBackErrorMessage("Rollback action for Import Cloud operation failed. "
                                             "Some leavings may exist on the local disk or in the Cloud.");
@@ -1597 +1607 @@
          */
 
-        {
+        { /** @todo r=bird: Pointless {}. */
             if (!fKeepDownloadedObject)
             {
+                ErrorInfoKeeper eik;    /* save the error info */
+                HRESULT const hrcSaved = hrc;
+
                 /* small explanation here, the image here points out to the whole downloaded object (not to the image only)
                  * filled during the first cloud import stage (in the ICloudClient::importInstance()) */
@@ -1622 +1635 @@
                     }
                 }
-            }
-        }
-
-        /* Because during the rollback phase the hrc may have the good result
-         * Thus we restore the original error in the case when the rollback phase was successful
-         * Otherwise we return not the original error but the last error in the rollback phase */
-         hrc = original_hrc;
+
+                /* Because during the rollback phase the hrc may have the good result
+                 * Thus we restore the original error in the case when the rollback phase was successful
+                 * Otherwise we return not the original error but the last error in the rollback phase */
+                hrc = hrcSaved;
+            }
+        }
     }
     else