Changeset 79133 in vbox

Timestamp:

Jun 13, 2019 3:27:38 PM (6 years ago)

Author:

vboxsync

Message:

tdAddGuestCtrl.py,Main: Added @todos for incorrect directoryOpen behavior. bugref:9320

Location:

trunk/src/VBox

Files:

• Main/idl/VirtualBox.xidl (modified) (1 diff)
• Main/src-client/GuestDirectoryImpl.cpp (modified) (1 diff)
• Main/src-client/GuestSessionImpl.cpp (modified) (3 diffs)
• ValidationKit/tests/additions/tdAddGuestCtrl.py (modified) (2 diffs)

trunk/src/VBox/Main/idl/VirtualBox.xidl

@@ -12921 +12921 @@
           may change in the future.</note>
 
+        <note>One idiosyncrasy of the current implementation is that you will NOT
+          get VBOX_E_OBJECT_NOT_FOUND returned here if the directory doesn't exist.
+          Instead the read function will fail with VBOX_E_IPRT_ERROR.  This will
+          be fixed soon.</note>
+
         <result name="VBOX_E_OBJECT_NOT_FOUND">
           Directory to open was not found.

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

@@ -104 +104 @@
          * Note: No guest rc available because operation is asynchronous.
          */
-        vrc = mData.mProcessTool.init(mSession, procInfo,
-                                      true /* Async */, NULL /* Guest rc */);
-    }
-
+        vrc = mData.mProcessTool.init(mSession, procInfo, true /* Async */, NULL /* Guest rc */);
+
+/** @todo r=bird: IGuest::directoryOpen need to fail if the directory doesn't
+ *        exist like it is documented to do.  It seems this async approach or
+ *        something is delaying such errors till GuestDirectory::read() is
+ *        called, which is clearly messed up.
+ */
+    }
+
+    /* Confirm a successful initialization when it's the case. */
     if (RT_SUCCESS(vrc))
-    {
-        /* Confirm a successful initialization when it's the case. */
         autoInitSpan.setSucceeded();
-        return vrc;
-    }
     else
         autoInitSpan.setFailed();
-
     return vrc;
 }

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

@@ -1131 +1131 @@
     HRESULT hr = pDirectory.createObject();
     if (FAILED(hr))
-        return VERR_COM_UNEXPECTED;
+        return Global::vboxStatusCodeFromCOM(hr);
 
     /* Register a new object ID. */
     uint32_t idObject;
-    int rc = i_objectRegister(pDirectory, SESSIONOBJECTTYPE_DIRECTORY, &idObject);
-    if (RT_FAILURE(rc))
+    int vrc = i_objectRegister(pDirectory, SESSIONOBJECTTYPE_DIRECTORY, &idObject);
+    if (RT_FAILURE(vrc))
     {
         pDirectory.setNull();
-        return rc;
+        return vrc;
     }
 
@@ -1145 +1145 @@
     AssertPtr(pConsole);
 
-    int vrc = pDirectory->init(pConsole, this /* Parent */, idObject, openInfo);
+    vrc = pDirectory->init(pConsole, this /* Parent */, idObject, openInfo);
     if (RT_FAILURE(vrc))
         return vrc;
@@ -3426 +3426 @@
     openInfo.mFlags = fFlags;
 
-    ComObjPtr <GuestDirectory> pDirectory; int rcGuest;
+    ComObjPtr<GuestDirectory> pDirectory; int rcGuest;
     int vrc = i_directoryOpen(openInfo, pDirectory, &rcGuest);
     if (RT_SUCCESS(vrc))

trunk/src/VBox/ValidationKit/tests/additions/tdAddGuestCtrl.py

@@ -1236 +1236 @@
         cOthers  = 0;    # Other files.
 
-        #
+        ##
+        ## @todo r=bird: Unlike fileOpen, directoryOpen will not fail if the directory does not exist.
+        ##       This is of course a bug in the implementation, as it is documented to return
+        ##       VBOX_E_OBJECT_NOT_FOUND or VBOX_E_IPRT_ERROR!
+        ##
+
         # Open the directory:
-        #
-        # Note! Unlike fileOpen, directoryOpen will not fail if the directory does not exist.
-        #       Looks like it won't do nothing till you read from it.
-        #
         #reporter.log2('Directory="%s", filter="%s", fFlags="%s"' % (sCurDir, sFilter, fFlags));
         try:
@@ -1255 +1256 @@
             except Exception as oXcpt:
                 if vbox.ComError.notEqual(oXcpt, vbox.ComError.VBOX_E_OBJECT_NOT_FOUND):
+                    ##
+                    ## @todo r=bird: Change this to reporter.errorXcpt() once directoryOpen() starts
+                    ##       working the way it is documented.
+                    ##
                     reporter.maybeErrXcpt(fIsError, 'Error reading directory "%s":' % (sCurDir,)); # See above why 'maybe'.
                     fRc = False;