Changeset 74495 in vbox

Timestamp:

Sep 27, 2018 12:08:01 PM (6 years ago)

Author:

vboxsync

Message:

DnD/Main: Cleaned up / documented error handling for GuestDnDSource::i_receiveDataThreadTask() and GuestDnDTarget::i_sendDataThreadTask().

Location:

trunk/src/VBox/Main/src-client

Files:

• GuestDnDSourceImpl.cpp (modified) (7 diffs)
• GuestDnDTargetImpl.cpp (modified) (5 diffs)

trunk/src/VBox/Main/src-client/GuestDnDSourceImpl.cpp

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2014-2017 Oracle Corporation
+ * Copyright (C) 2014-2018 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -1066 +1066 @@
 
     int vrc = pThis->i_receiveData(pTask->getCtx(), RT_INDEFINITE_WAIT /* msTimeout */);
-    AssertRC(vrc);
-/** @todo
- *
- *  r=bird: What happens with @a vrc?
- *
- */
+    if (RT_FAILURE(vrc)) /* In case we missed some error handling within i_receiveData(). */
+    {
+        AssertFailed();
+        LogRel(("DnD: Receiving data from guest failed with %Rrc\n", vrc));
+    }
 
     AutoWriteLock alock(pThis COMMA_LOCKVAL_SRC_POS);
@@ -1079 +1078 @@
         pThis->mDataBase.m_cTransfersPending--;
 
-    LogFlowFunc(("pSource=%p vrc=%Rrc (ignored)\n", (GuestDnDSource *)pThis, vrc));
+    LogFlowFunc(("pSource=%p, vrc=%Rrc (ignored)\n", (GuestDnDSource *)pThis, vrc));
 }
 
@@ -1160 +1159 @@
     if (RT_FAILURE(rc))
     {
-        if (rc == VERR_CANCELLED)
-        {
-            int rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED);
+        if (rc == VERR_CANCELLED) /* Transfer was cancelled by the host. */
+        {
+            /*
+             * Now that we've cleaned up tell the guest side to cancel.
+             * This does not imply we're waiting for the guest to react, as the
+             * host side never must depend on anything from the guest.
+             */
+            int rc2 = sendCancel();
             AssertRC(rc2);
 
-            rc2 = sendCancel();
+            rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED);
             AssertRC(rc2);
         }
@@ -1174 +1178 @@
             AssertRC(rc2);
         }
+
+        rc = VINF_SUCCESS; /* The error was handled by the setProgress() calls above. */
     }
 
@@ -1278 +1284 @@
     if (RT_FAILURE(rc))
     {
+        int rc2 = droppedFiles.Rollback();
+        if (RT_FAILURE(rc2))
+            LogRel(("DnD: Deleting left over temporary files failed (%Rrc), please remove directory '%s' manually\n",
+                    rc2, droppedFiles.GetDirAbs()));
+
         if (rc == VERR_CANCELLED)
         {
-            int rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED);
+            /*
+             * Now that we've cleaned up tell the guest side to cancel.
+             * This does not imply we're waiting for the guest to react, as the
+             * host side never must depend on anything from the guest.
+             */
+            rc2 = sendCancel();
             AssertRC(rc2);
 
-            rc2 = sendCancel();
+            rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED);
             AssertRC(rc2);
         }
@@ -1292 +1308 @@
             AssertRC(rc2);
         }
-    }
-
-    if (RT_FAILURE(rc))
-    {
-        int rc2 = droppedFiles.Rollback();
-        if (RT_FAILURE(rc2))
-            LogRel(("DnD: Deleting left over temporary files failed (%Rrc). Please remove directory manually: %s\n",
-                    rc2, droppedFiles.GetDirAbs()));
+
+        rc = VINF_SUCCESS; /* The error was handled by the setProgress() calls above. */
     }
 

trunk/src/VBox/Main/src-client/GuestDnDTargetImpl.cpp

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2014-2017 Oracle Corporation
+ * Copyright (C) 2014-2018 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -567 +567 @@
 {
     LogFlowFunc(("pTask=%p\n", pTask));
-
     AssertPtrReturnVoid(pTask);
 
@@ -578 +577 @@
 
     int vrc = pThis->i_sendData(pTask->getCtx(), RT_INDEFINITE_WAIT /* msTimeout */);
-    NOREF(vrc);
-/** @todo
- *
- *  r=bird: What happens with @a vrc?
- *
- */
+    if (RT_FAILURE(vrc)) /* In case we missed some error handling within i_sendData(). */
+    {
+        AssertFailed();
+        LogRel(("DnD: Sending data to guest failed with %Rrc\n", vrc));
+    }
 
     AutoWriteLock alock(pThis COMMA_LOCKVAL_SRC_POS);
@@ -591 +589 @@
         pThis->mDataBase.m_cTransfersPending--;
 
-    LogFlowFunc(("pTarget=%p vrc=%Rrc (ignored)\n", (GuestDnDTarget *)pThis, vrc));
+    LogFlowFunc(("pTarget=%p, vrc=%Rrc (ignored)\n", (GuestDnDTarget *)pThis, vrc));
 }
 
@@ -1419 +1417 @@
     if (RT_FAILURE(rc))
     {
-        if (rc == VERR_CANCELLED)
-            pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED, VINF_SUCCESS);
+        if (rc == VERR_CANCELLED) /* Transfer was cancelled by the host. */
+        {
+            /*
+             * Now that we've cleaned up tell the guest side to cancel.
+             * This does not imply we're waiting for the guest to react, as the
+             * host side never must depend on anything from the guest.
+             */
+            int rc2 = sendCancel();
+            AssertRC(rc2);
+
+            rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_CANCELLED, VINF_SUCCESS);
+            AssertRC(rc2);
+        }
         else if (rc != VERR_GSTDND_GUEST_ERROR) /* Guest-side error are already handled in the callback. */
-            pCtx->mpResp->setProgress(100, DND_PROGRESS_ERROR, rc,
-                                      GuestDnDTarget::i_hostErrorToString(rc));
-    }
-
-    /*
-     * Now that we've cleaned up tell the guest side to cancel.
-     * This does not imply we're waiting for the guest to react, as the
-     * host side never must depend on anything from the guest.
-     */
-    if (rc == VERR_CANCELLED)
-    {
-        int rc2 = sendCancel();
-        AssertRC(rc2);
+        {
+            int rc2 = pCtx->mpResp->setProgress(100, DND_PROGRESS_ERROR, rc,
+                                                GuestDnDTarget::i_hostErrorToString(rc));
+            AssertRC(rc2);
+        }
+
+        rc = VINF_SUCCESS; /* The error was handled by the setProgress() calls above. */
     }