[vbox-dev] Fixing Recursive Copy

[John Chatham]

I've dropped some patches for this onto the forums - see here:  https://forums.virtualbox.org/viewtopic.php?f=10&t=64285

One of them's already on trunk (thanks, Frank Mehnert!), but it's been suggested that I should post this stuff to the mailing list as well, so here you go.

--- src/VBox/Main/src-client/GuestDirectoryImpl.cpp	(original code)
+++ src/VBox/Main/src-client/GuestDirectoryImpl.cpp	(working copy)
@@ -89,7 +89,7 @@
     {
         /* Start the directory process on the guest. */
         GuestProcessStartupInfo procInfo;
-        procInfo.mName      = Utf8StrFmt(tr("Reading directory \"%s\"", openInfo.mPath.c_str()));
+        procInfo.mName      = Utf8StrFmt(tr("Reading directory \"%s\""), openInfo.mPath.c_str());
         procInfo.mCommand   = Utf8Str(VBOXSERVICE_TOOL_LS);
         procInfo.mTimeoutMS = 5 * 60 * 1000; /* 5 minutes timeout. */
         procInfo.mFlags     = ProcessCreateFlag_WaitForStdOut;



--- src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp	(original code)
+++ src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp	(working copy)
@@ -1776,11 +1776,13 @@
     if (fOnGuest)
     {
         BOOL fDirExists = FALSE;
+        /* This function is bugged - it always returns an error if the directory does not exist. */
         HRESULT rc = pContext->pCmdCtx->pGuestSession->DirectoryExists(Bstr(pszDir).raw(), &fDirExists);
-        if (FAILED(rc))
+        *fExists = fDirExists ? true : false;
+        /* If it set fDirExists and returned an error, then we want to error.  Otherwise, we
+         * need to return success with fExists set to false. */
+        if(fDirExists && FAILED(rc))
             vrc = ctrlPrintError(pContext->pCmdCtx->pGuestSession, COM_IIDOF(IGuestSession));
-        else
-            *fExists = fDirExists ? true : false;
     }
     else
         *fExists = RTDirExists(pszDir);
@@ -1983,9 +1985,15 @@
     if (pContext->pCmdCtx->fVerbose)
         RTPrintf("Processing host directory: %s\n", szCurDir);
 
-    /* Flag indicating whether the current directory was created on the
-     * target or not. */
-    bool fDirCreated = false;
+    /* Always create directory, otherwise we'll fail to copy empty directories. */
+    char *pszDestDir;
+    vrc = ctrlCopyTranslatePath(pszSource, szCurDir,
+                               pszDest, &pszDestDir);
+    if (RT_SUCCESS(vrc))
+    {
+        vrc = ctrlCopyDirCreate(pContext, pszDestDir);
+        RTStrFree(pszDestDir);
+    }
 
     /*
      * Open directory without a filter - RTDirOpenFiltered unfortunately
@@ -2072,36 +2080,19 @@
                     if (pContext->pCmdCtx->fVerbose)
                         RTPrintf("File: %s\n", DirEntry.szName);
 
-                    if (!fDirCreated)
+                    char *pszFileSource = RTPathJoinA(szCurDir, DirEntry.szName);
+                    if (pszFileSource)
                     {
-                        char *pszDestDir;
-                        vrc = ctrlCopyTranslatePath(pszSource, szCurDir,
-                                                    pszDest, &pszDestDir);
+                        char *pszFileDest;
+                        vrc = ctrlCopyTranslatePath(pszSource, pszFileSource,
+                                                   pszDest, &pszFileDest);
                         if (RT_SUCCESS(vrc))
                         {
-                            vrc = ctrlCopyDirCreate(pContext, pszDestDir);
-                            RTStrFree(pszDestDir);
-
-                            fDirCreated = true;
-                        }
-                    }
-
-                    if (RT_SUCCESS(vrc))
-                    {
-                        char *pszFileSource = RTPathJoinA(szCurDir, DirEntry.szName);
-                        if (pszFileSource)
-                        {
-                            char *pszFileDest;
-                            vrc = ctrlCopyTranslatePath(pszSource, pszFileSource,
-                                                       pszDest, &pszFileDest);
-                            if (RT_SUCCESS(vrc))
-                            {
-                                vrc = ctrlCopyFileToDest(pContext, pszFileSource,
-                                                        pszFileDest, 0 /* Flags */);
-                                RTStrFree(pszFileDest);
-                            }
-                            RTStrFree(pszFileSource);
+                            vrc = ctrlCopyFileToDest(pContext, pszFileSource,
+                                                    pszFileDest, 0 /* Flags */);
+                            RTStrFree(pszFileDest);
                         }
+                        RTStrFree(pszFileSource);
                     }
                     break;
                 }
@@ -2155,9 +2146,18 @@
     if (pContext->pCmdCtx->fVerbose)
         RTPrintf("Processing guest directory: %s\n", szCurDir);
 
-    /* Flag indicating whether the current directory was created on the
-     * target or not. */
-    bool fDirCreated = false;
+    /* Create directory here, otherwise we will fail to copy empty directories. */
+    char *pszDestDir;
+    vrc = ctrlCopyTranslatePath(pszSource, szCurDir,
+                                pszDest, &pszDestDir);
+    if (RT_SUCCESS(vrc))
+    {
+        vrc = ctrlCopyDirCreate(pContext, pszDestDir);
+        RTStrFree(pszDestDir);
+    }
+    if (RT_FAILURE(vrc))
+        return vrc;
+
     SafeArray<DirectoryOpenFlag_T> dirOpenFlags; /* No flags supported yet. */
     ComPtr<IGuestDirectory> pDirectory;
     HRESULT rc = pContext->pCmdCtx->pGuestSession->DirectoryOpen(Bstr(szCurDir).raw(), Bstr(pszFilter).raw(),
@@ -2242,39 +2242,22 @@
                 if (pContext->pCmdCtx->fVerbose)
                     RTPrintf("File: %s\n", strFile.c_str());
 
-                if (!fDirCreated)
+                char *pszFileSource = RTPathJoinA(szCurDir, strFile.c_str());
+                if (pszFileSource)
                 {
-                    char *pszDestDir;
-                    vrc = ctrlCopyTranslatePath(pszSource, szCurDir,
-                                                pszDest, &pszDestDir);
+                    char *pszFileDest;
+                    vrc = ctrlCopyTranslatePath(pszSource, pszFileSource,
+                                               pszDest, &pszFileDest);
                     if (RT_SUCCESS(vrc))
                     {
-                        vrc = ctrlCopyDirCreate(pContext, pszDestDir);
-                        RTStrFree(pszDestDir);
-
-                        fDirCreated = true;
+                        vrc = ctrlCopyFileToDest(pContext, pszFileSource,
+                                                pszFileDest, 0 /* Flags */);
+                        RTStrFree(pszFileDest);
                     }
+                    RTStrFree(pszFileSource);
                 }
-
-                if (RT_SUCCESS(vrc))
-                {
-                    char *pszFileSource = RTPathJoinA(szCurDir, strFile.c_str());
-                    if (pszFileSource)
-                    {
-                        char *pszFileDest;
-                        vrc = ctrlCopyTranslatePath(pszSource, pszFileSource,
-                                                   pszDest, &pszFileDest);
-                        if (RT_SUCCESS(vrc))
-                        {
-                            vrc = ctrlCopyFileToDest(pContext, pszFileSource,
-                                                    pszFileDest, 0 /* Flags */);
-                            RTStrFree(pszFileDest);
-                        }
-                        RTStrFree(pszFileSource);
-                    }
-                    else
-                        vrc = VERR_NO_MEMORY;
-                }
+                else
+                    vrc = VERR_NO_MEMORY;
                 break;
             }
 
@@ -2293,6 +2276,7 @@
         switch (rc)
         {
             case E_ABORT: /* No more directory entries left to process. */
+            case VBOX_E_OBJECT_NOT_FOUND: /* Also indicates no more entries to process. */
                 break;
 
             case VBOX_E_FILE_ERROR: /* Current entry cannot be accessed to