lib/file: File_Rename becomes File_Move

The File_Rename function is really the equivalent of the
unix/Linux "mv" command - it may atomically rename or copy the
file. Unfortunately there is no nice way to know what actually
happened, as cleanup may be necessary upon failure. Fix this.

While we're at it, make the GetLastError/errno make sense.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>

diff --git a/open-vm-tools/lib/file/file.c b/open-vm-tools/lib/file/file.c
index 750d3a0dfbf028fe6045e2bc9b2693164e4b32d1..f6d6f083216ad637af196495d4022dd3a64b4e4a 100644 (file)
--- a/open-vm-tools/lib/file/file.c
+++ b/open-vm-tools/lib/file/file.c
@@ -1386,8 +1386,8 @@ File_MakeTemp(ConstUnicode tag,  // IN (OPT):
  *      end of the 'src' file to the current position in the 'dst' file
  *
  * Results:
- *      TRUE on success
- *      FALSE on failure
+ *      TRUE   success
+ *      FALSE  failure
  *
  * Side effects:
  *      The current position in the 'src' file and the 'dst' file are modified
@@ -1399,6 +1399,7 @@ Bool
 File_CopyFromFdToFd(FileIODescriptor src,  // IN:
                     FileIODescriptor dst)  // IN:
 {
+   Err_Number err;
    FileIOResult fretR;
 
    do {
@@ -1408,17 +1409,25 @@ File_CopyFromFdToFd(FileIODescriptor src,  // IN:
 
       fretR = FileIO_Read(&src, buf, sizeof(buf), &actual);
       if (!FileIO_IsSuccess(fretR) && (fretR != FILEIO_READ_ERROR_EOF)) {
+         err = Err_Errno();
+
          Msg_Append(MSGID(File.CopyFromFdToFd.read.failure)
                                "Read error: %s.\n\n", FileIO_MsgError(fretR));
 
+         Err_SetErrno(err);
+
          return FALSE;
       }
 
       fretW = FileIO_Write(&dst, buf, actual, NULL);
       if (!FileIO_IsSuccess(fretW)) {
+         err = Err_Errno();
+
          Msg_Append(MSGID(File.CopyFromFdToFd.write.failure)
                               "Write error: %s.\n\n", FileIO_MsgError(fretW));
 
+         Err_SetErrno(err);
+
          return FALSE;
       }
    } while (fretR != FILEIO_READ_ERROR_EOF);
@@ -1455,9 +1464,10 @@ File_CopyFromFdToName(FileIODescriptor src,  // IN:
                       ConstUnicode dstName,  // IN:
                       int dstDispose)        // IN:
 {
-   FileIODescriptor dst;
+   Bool success;
+   Err_Number err;
    FileIOResult fret;
-   Bool result;
+   FileIODescriptor dst;
 
    ASSERT(dstName);
 
@@ -1465,30 +1475,43 @@ File_CopyFromFdToName(FileIODescriptor src,  // IN:
 
    fret = File_CreatePrompt(&dst, dstName, 0, dstDispose);
    if (!FileIO_IsSuccess(fret)) {
+      err = Err_Errno();
+
       if (fret != FILEIO_CANCELLED) {
          Msg_Append(MSGID(File.CopyFromFdToName.create.failure)
                     "Unable to create a new '%s' file: %s.\n\n",
                     UTF8(dstName), FileIO_MsgError(fret));
       }
 
+      Err_SetErrno(err);
+
       return FALSE;
    }
 
-   result = File_CopyFromFdToFd(src, dst);
+   success = File_CopyFromFdToFd(src, dst);
+
+   err = Err_Errno();
 
    if (FileIO_Close(&dst) != 0) {
+      if (success) {  // Report close failure when there isn't another error
+         err =  Err_Errno();
+      }
+
       Msg_Append(MSGID(File.CopyFromFdToName.close.failure)
                  "Unable to close the '%s' file: %s.\n\n", UTF8(dstName),
                  Msg_ErrString());
-      result = FALSE;
+
+      success = FALSE;
    }
 
-   if (result == FALSE) {
+   if (!success) {
       /* The copy failed: ensure the destination file is removed */
       File_Unlink(dstName);
    }
 
-   return result;
+   Err_SetErrno(err);
+
+   return success;
 }
 
 
@@ -1518,8 +1541,8 @@ File_CreatePrompt(FileIODescriptor *file,  // OUT:
                   int access,              // IN:
                   int prompt)              // IN:
 {
-   FileIOOpenAction action;
    FileIOResult fret;
+   FileIOOpenAction action;
 
    ASSERT(file);
    ASSERT(pathName);
@@ -1529,12 +1552,13 @@ File_CreatePrompt(FileIODescriptor *file,  // OUT:
    while ((fret = FileIO_Open(file, pathName, FILEIO_OPEN_ACCESS_WRITE | access,
                              action)) == FILEIO_OPEN_ERROR_EXIST) {
       static Msg_String const buttons[] = {
-         {BUTTONID(file.create.retry) "Retry"},
+         {BUTTONID(file.create.retry)     "Retry"},
          {BUTTONID(file.create.overwrite) "Overwrite"},
-         {BUTTONID(cancel) "Cancel"},
+         {BUTTONID(cancel)                "Cancel"},
          {NULL}
       };
       int answer;
+      Err_Number err = Err_Errno();
 
       answer = (prompt != -1) ? prompt : Msg_Question(buttons, 2,
          MSGID(File.CreatePrompt.question)
@@ -1544,6 +1568,9 @@ File_CreatePrompt(FileIODescriptor *file,  // OUT:
          "to another location, select Retry.\n"
          "To cancel the operation, select Cancel.\n",
          UTF8(pathName));
+
+      Err_SetErrno(err);
+
       if (answer == 2) {
          fret = FILEIO_CANCELLED;
          break;
@@ -1586,9 +1613,10 @@ File_CopyFromNameToName(ConstUnicode srcName,  // IN:
                         ConstUnicode dstName,  // IN:
                         int dstDispose)        // IN:
 {
-   FileIODescriptor src;
+   Bool success;
+   Err_Number err;
    FileIOResult fret;
-   Bool result;
+   FileIODescriptor src;
 
    ASSERT(srcName);
    ASSERT(dstName);
@@ -1597,23 +1625,36 @@ File_CopyFromNameToName(ConstUnicode srcName,  // IN:
 
    fret = FileIO_Open(&src, srcName, FILEIO_OPEN_ACCESS_READ, FILEIO_OPEN);
    if (!FileIO_IsSuccess(fret)) {
+      err = Err_Errno();
+
       Msg_Append(MSGID(File.CopyFromNameToName.open.failure)
                  "Unable to open the '%s' file for read access: %s.\n\n",
                  UTF8(srcName), FileIO_MsgError(fret));
 
+      Err_SetErrno(err);
+
       return FALSE;
    }
 
-   result = File_CopyFromFdToName(src, dstName, dstDispose);
+   success = File_CopyFromFdToName(src, dstName, dstDispose);
+
+   err = Err_Errno();
    
    if (FileIO_Close(&src) != 0) {
+      if (success) {  // Report close failure when there isn't another error
+         err =  Err_Errno();
+      }
+
       Msg_Append(MSGID(File.CopyFromNameToName.close.failure)
                  "Unable to close the '%s' file: %s.\n\n", UTF8(srcName),
                  Msg_ErrString());
-      result = FALSE;
+
+      success = FALSE;
    }
 
-   return result;
+   Err_SetErrno(err);
+
+   return success;
 }
 
 /*
@@ -1640,10 +1681,11 @@ File_CopyFromFd(FileIODescriptor src,     // IN:
                 ConstUnicode dstName,     // IN:
                 Bool overwriteExisting)   // IN:
 {
+   Bool success;
+   Err_Number err;
+   FileIOResult fret;
    FileIODescriptor dst;
    FileIOOpenAction action;
-   FileIOResult fret;
-   Bool result;
 
    ASSERT(dstName);
 
@@ -1654,28 +1696,41 @@ File_CopyFromFd(FileIODescriptor src,     // IN:
 
    fret = FileIO_Open(&dst, dstName, FILEIO_OPEN_ACCESS_WRITE, action);
    if (!FileIO_IsSuccess(fret)) {
+      err = Err_Errno();
+
       Msg_Append(MSGID(File.CopyFromFdToName.create.failure)
                  "Unable to create a new '%s' file: %s.\n\n", UTF8(dstName),
                  FileIO_MsgError(fret));
 
+      Err_SetErrno(err);
+
       return FALSE;
    }
 
-   result = File_CopyFromFdToFd(src, dst);
+   success = File_CopyFromFdToFd(src, dst);
+
+   err = Err_Errno();
 
    if (FileIO_Close(&dst) != 0) {
+      if (success) {  // Report close failure when there isn't another error
+         err =  Err_Errno();
+      }
+
       Msg_Append(MSGID(File.CopyFromFdToName.close.failure)
                  "Unable to close the '%s' file: %s.\n\n", UTF8(dstName),
                  Msg_ErrString());
-      result = FALSE;
+
+      success = FALSE;
    }
 
-   if (result == FALSE) {
+   if (!success) {
       /* The copy failed: ensure the destination file is removed */
       File_Unlink(dstName);
    }
 
-   return result;
+   Err_SetErrno(err);
+
+   return success;
 }
 
 
@@ -1704,9 +1759,10 @@ File_Copy(ConstUnicode srcName,    // IN:
           ConstUnicode dstName,    // IN:
           Bool overwriteExisting)  // IN:
 {
-   FileIODescriptor src;
+   Bool success;
+   Err_Number err;
    FileIOResult fret;
-   Bool result;
+   FileIODescriptor src;
 
    ASSERT(srcName);
    ASSERT(dstName);
@@ -1715,35 +1771,52 @@ File_Copy(ConstUnicode srcName,    // IN:
 
    fret = FileIO_Open(&src, srcName, FILEIO_OPEN_ACCESS_READ, FILEIO_OPEN);
    if (!FileIO_IsSuccess(fret)) {
+      err = Err_Errno();
+
       Msg_Append(MSGID(File.Copy.open.failure)
                  "Unable to open the '%s' file for read access: %s.\n\n",
                  UTF8(srcName), FileIO_MsgError(fret));
 
+      Err_SetErrno(err);
+
       return FALSE;
    }
 
-   result = File_CopyFromFd(src, dstName, overwriteExisting);
+   success = File_CopyFromFd(src, dstName, overwriteExisting);
+
+   err = Err_Errno();
    
    if (FileIO_Close(&src) != 0) {
+      if (success) {  // Report close failure when there isn't another error
+         err =  Err_Errno();
+      }
+
       Msg_Append(MSGID(File.Copy.close.failure)
                  "Unable to close the '%s' file: %s.\n\n", UTF8(srcName),
                  Msg_ErrString());
-      result = FALSE;
+
+      success = FALSE;
    }
 
-   return result;
+   Err_SetErrno(err);
+
+   return success;
 }
 
+
 /*
  *----------------------------------------------------------------------
  *
- * File_Rename --
+ * File_Move --
  *
- *      Renames a source to a destination file.
- *      Will copy the file if necessary
+ *      Moves a file from one place to the other as efficiently as possible.
+ *      This can be used to rename a file but, since file copying may be
+ *      necessary, there is no assurance of atomicity. For efficiency
+ *      purposes copying only results if the native rename ability fails.
  *
  * Results:
- *      TRUE if succeeded FALSE otherwise
+ *      TRUE   succeeded
+ *      FALSE  otherwise
  *      
  * Side effects:
  *      src file is no more, but dst file exists
@@ -1752,20 +1825,33 @@ File_Copy(ConstUnicode srcName,    // IN:
  */
 
 Bool 
-File_Rename(ConstUnicode oldFile,  // IN:
-            ConstUnicode newFile)  // IN:
+File_Move(ConstUnicode oldFile,  // IN:
+          ConstUnicode newFile,  // IN:
+          Bool *asRename)        // OUT: result occurred due to rename/copy
 {
-   Bool ret = TRUE;
+   Bool ret;
+   Bool duringRename;
 
-   if (FileRename(oldFile, newFile) != 0) {
-      /* overwrite the file if it exists */
-      if (File_Copy(oldFile, newFile, TRUE)) {
-         File_Unlink(oldFile);
+   if (FileRename(oldFile, newFile) == 0) {
+      duringRename = TRUE;
+      ret = TRUE;
+      Err_SetErrno(0);
+   } else {
+      duringRename = FALSE;
+
+      if (File_Copy(oldFile, newFile, TRUE)) {  // Allow overwrite
+         File_Unlink(oldFile);  // Explicitly ignore errors
+         ret = TRUE;
+         Err_SetErrno(0);
       } else {
          ret = FALSE;
       }
    }
 
+   if (asRename) {
+      *asRename = duringRename;
+   }
+
    return ret;
 }
 

diff --git a/open-vm-tools/lib/include/file.h b/open-vm-tools/lib/include/file.h
index 20576cb81c904f2433a4208fcd06c267efbe2e90..90ece13a6a362d1a26ac842beb74533ee288504b 100644 (file)
--- a/open-vm-tools/lib/include/file.h
+++ b/open-vm-tools/lib/include/file.h
@@ -36,6 +36,7 @@ extern "C"{
 
 #include "fileIO.h"
 #include "unicodeTypes.h"
+#include "err.h"
 
 #ifdef N_PLAT_NLM
 #define FILE_MAXPATH   256
@@ -273,8 +274,9 @@ Bool File_CopyFromNameToName(ConstUnicode srcName,
 Bool File_Replace(ConstUnicode oldFile, 
                   ConstUnicode newFile);
 
-Bool File_Rename(ConstUnicode oldFile, 
-                 ConstUnicode newFile);
+Bool File_Move(ConstUnicode oldFile, 
+               ConstUnicode newFile,
+               Bool *asRename);
 
 void File_Rotate(const char *fileName,
                  int n,

diff --git a/open-vm-tools/services/plugins/vix/vixTools.c b/open-vm-tools/services/plugins/vix/vixTools.c
index 3bb938be541d58441e606d6c479ac2f1344c64f7..77529f277b3dcf84b65dfbf3833d461e2893824b 100644 (file)
--- a/open-vm-tools/services/plugins/vix/vixTools.c
+++ b/open-vm-tools/services/plugins/vix/vixTools.c
@@ -3698,7 +3698,7 @@ VixToolsMoveObject(VixCommandRequestHeader *requestMsg)        // IN
 #endif
 
    /*
-    * pre-check the dest arg -- File_Rename() will return
+    * pre-check the dest arg -- File_Move() will return
     * diff err codes depending on OS, so catch it up front (bug 133165)
     */
    if (File_IsDirectory(destFilePathName)) {
@@ -3768,7 +3768,7 @@ VixToolsMoveObject(VixCommandRequestHeader *requestMsg)        // IN
       }
 
       /*
-       * In case of moving a directory, File_Rename() returns different
+       * In case of moving a directory, File_Move() returns different
        * errors on different Guest Os if the destination file path points
        * to an existing file. We should catch them upfront and report them
        * to the user.
@@ -3782,10 +3782,10 @@ VixToolsMoveObject(VixCommandRequestHeader *requestMsg)        // IN
       }
    }
 
-   success = File_Rename(srcFilePathName, destFilePathName);
+   success = File_Move(srcFilePathName, destFilePathName, NULL);
    if (!success) {
       err = FoundryToolsDaemon_TranslateSystemErr();
-      Debug("%s: File_Rename failed.\n", __FUNCTION__);
+      Debug("%s: File_Move failed.\n", __FUNCTION__);
       goto abort;
    }
 

diff --git a/open-vm-tools/toolbox/debugStdio.c b/open-vm-tools/toolbox/debugStdio.c
index c8ead38bfdb66222f8fd87cc6ddc646d6e4c5e71..642be0ecba66f13ebdfcb3737206ca4e128cdcd7 100644 (file)
--- a/open-vm-tools/toolbox/debugStdio.c
+++ b/open-vm-tools/toolbox/debugStdio.c
@@ -103,7 +103,7 @@ Debug_EnableToFile(const char *file,      // IN
       if (bakFile &&
           !File_IsDirectory(bakFile) &&
           0 == File_UnlinkIfExists(bakFile)) {  // remove old back up file.
-         File_Rename(file, bakFile);
+         File_Move(file, bakFile, NULL);
       }
       free(bakFile);
    }