lib/file: pick up some review comments

Picking up a few review comments from the last lib/file change
review.

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 8d711bb34c53cb9b27532d3829b6e2c4e3cf1e0a..6cffaa7e56993f25583e84bb9218ff443c749e13 100644 (file)
--- a/open-vm-tools/lib/file/file.c
+++ b/open-vm-tools/lib/file/file.c
@@ -48,9 +48,6 @@
 #include "str.h"
 #include "msg.h"
 #include "random.h"
-#if defined(VMX86_VMX)
-#include "vthreadBase.h"
-#endif
 #include "uuid.h"
 #include "config.h"
 #include "posix.h"
@@ -193,9 +190,10 @@ File_GetFilePermissions(ConstUnicode pathName,  // IN:
    }
 
    *mode = fileData.fileMode;
+
 #if defined(_WIN32)
       /*
-       *  On Win32 implementation of FileAttributes does not return execution
+       * On Win32 implementation of FileAttributes does not return execution
        * bit.
        */
 
@@ -203,6 +201,7 @@ File_GetFilePermissions(ConstUnicode pathName,  // IN:
          *mode |= S_IXUSR;
       }
 #endif
+
    return TRUE;
 }
 
@@ -2079,11 +2078,13 @@ File_PrependToPath(const char *searchPath,   // IN
          }
          break;
       }
+
       if (!next) {
          break;
       }
       path = next + 1;
    }
+
    return newPath;
 }
 
@@ -2314,12 +2315,10 @@ File_ReplaceExtension(ConstUnicode pathName,      // IN:
  */
 
 char *
-File_ExpandAndCheckDir(const char *dirName)
+File_ExpandAndCheckDir(const char *dirName)  // IN:
 {
-   char *edirName;
-
    if (dirName != NULL) {
-      edirName = Util_ExpandString(dirName);
+      char *edirName = Util_ExpandString(dirName);
 
       if ((edirName != NULL) && FileIsWritableDir(edirName)) {
          size_t len = strlen(edirName) - 1;
@@ -2336,6 +2335,63 @@ File_ExpandAndCheckDir(const char *dirName)
 }
 
 
+/*
+ *-----------------------------------------------------------------------------
+ *
+ * FileSimpleRandom --
+ *
+ *      Return a random number in the range of 0 and 2^32-1.
+ *
+ *     This isn't thread safe but it's more than good enough for the
+ *      purposes required of it.
+ *
+ * Results:
+ *      Random number is returned.
+ *
+ * Side Effects:
+ *      None.
+ *
+ *-----------------------------------------------------------------------------
+ */
+
+uint32
+FileSimpleRandom(void)
+{
+   static Atomic_Ptr atomic; /* Implicitly initialized to NULL. --mbellon */
+   char *context;
+
+   context = Atomic_ReadPtr(&atomic);
+
+   if (UNLIKELY(context == NULL)) {
+      void *p;
+      uint32 value;
+
+      /*
+       * Threads will hash up this RNG - this isn't officially thread safe
+       * which is just fine - but ensure that different processes have
+       * different answer streams.
+       */
+
+#if defined(_WIN32)
+      value = GetCurrentProcessId();
+#else
+      value = getpid();
+#endif
+
+      p = Random_QuickSeed(value);
+
+      if (Atomic_ReadIfEqualWritePtr(&atomic, NULL, p)) {
+         free(p);
+      }
+
+      context = Atomic_ReadPtr(&atomic);
+      ASSERT(context);
+   }
+
+   return Random_Quick(context);
+}
+
+
 /*
  *----------------------------------------------------------------------
  *
@@ -2366,38 +2422,12 @@ FileSleeper(uint32 msecMinSleepTime,  // IN:
    if (variance == 0) {
       msecActualSleepTime = msecMinSleepTime;
    } else {
-      int sample;
-      float fpRand;
-      static int32 rng;  // implicitly 0
-
-      if (rng == 0) {
-         int pid;
-
-#if defined(_WIN32)
-         pid = GetCurrentProcessId();
-#else
-         pid = getpid();
-#endif
-
-         rng = (pid << 16);
-
-#if defined(VMX86_VMX)
-         rng |= VThread_CurID();
-#endif
-      }
-
-      sample = FastRand(rng);
-      fpRand = ((float) sample) / ((float) MAX_INT32);
-      rng = sample;
+      float fpRand = ((float) FileSimpleRandom()) / ((float) ~((uint32) 0));
 
       msecActualSleepTime = msecMinSleepTime + (uint32) (fpRand * variance);
    }
 
-#if defined(_WIN32)
-   Sleep(msecActualSleepTime);
-#else
    usleep(1000 * msecActualSleepTime);
-#endif
 
    return msecActualSleepTime;
 }

diff --git a/open-vm-tools/lib/file/fileInt.h b/open-vm-tools/lib/file/fileInt.h
index 21b962792df31b4464bbe631d040ecbeeda2ba3c..dfdfc10be22a5eefed42286cffd890b6bc9d61cf 100644 (file)
--- a/open-vm-tools/lib/file/fileInt.h
+++ b/open-vm-tools/lib/file/fileInt.h
@@ -135,20 +135,20 @@ EXTERN int FileListDirectoryRetry(ConstUnicode pathName,
 #define FileCreateDirectory(a)  FileCreateDirectoryRetry((a), 0)
 #define FileRemoveDirectory(a)  FileRemoveDirectoryRetry((a), 0)
 
-#define FILE_MAX_WAIT_TIME 2000
+#define FILE_MAX_WAIT_TIME_MS 2000  // maximum wait time in milliseconds
 
 #define FileListDirectoryRobust(a, b) \
-                    FileListDirectoryRetry((a), FILE_MAX_WAIT_TIME, (b))
+                    FileListDirectoryRetry((a), FILE_MAX_WAIT_TIME_MS, (b))
 #define FileAttributesRobust(a, b) \
-                    FileAttributesRetry((a), FILE_MAX_WAIT_TIME, (b))
+                    FileAttributesRetry((a), FILE_MAX_WAIT_TIME_MS, (b))
 #define FileRenameRobust(a, b) \
-                    FileRenameRetry((a), (b), FILE_MAX_WAIT_TIME)
+                    FileRenameRetry((a), (b), FILE_MAX_WAIT_TIME_MS)
 #define FileDeletionRobust(a, b) \
-                    FileDeletionRetry((a), (b), FILE_MAX_WAIT_TIME)
+                    FileDeletionRetry((a), (b), FILE_MAX_WAIT_TIME_MS)
 #define FileCreateDirectoryRobust(a) \
-                    FileCreateDirectoryRetry((a), FILE_MAX_WAIT_TIME)
+                    FileCreateDirectoryRetry((a), FILE_MAX_WAIT_TIME_MS)
 #define FileRemoveDirectoryRobust(a) \
-                    FileRemoveDirectoryRetry((a), FILE_MAX_WAIT_TIME)
+                    FileRemoveDirectoryRetry((a), FILE_MAX_WAIT_TIME_MS)
 #else
 EXTERN char *FilePosixGetBlockDevice(char const *path);
 
@@ -210,6 +210,8 @@ typedef int FILELOCK_FILE_HANDLE;
 EXTERN uint32 FileSleeper(uint32 msecMinSleepTime,
                           uint32 msecMaxSleepTime);
 
+EXTERN uint32 FileSimpleRandom(void);
+
 EXTERN const char *FileLockGetMachineID(void);
 
 EXTERN char *FileLockGetExecutionID(void);

diff --git a/open-vm-tools/lib/file/fileLockPrimitive.c b/open-vm-tools/lib/file/fileLockPrimitive.c
index 4736cd53de9b315a2db60b2cd63de4656ac7cf76..f805274b19b914ed30beadccae4539d284dd24c4 100644 (file)
--- a/open-vm-tools/lib/file/fileLockPrimitive.c
+++ b/open-vm-tools/lib/file/fileLockPrimitive.c
@@ -1099,62 +1099,6 @@ NumberScan(ConstUnicode lockDir,      // IN:
 }
 
 
-/*
- *-----------------------------------------------------------------------------
- *
- * SimpleRandomNumber --
- *
- *      Return a random number in the range of 0 and 2^16-1.
- *
- * Results:
- *      Random number is returned.
- *
- * Side Effects:
- *      None.
- *
- *-----------------------------------------------------------------------------
- */
-
-static uint32
-SimpleRandomNumber(const char *machineID,    // IN:
-                   const char *executionID)  // IN:
-{
-   static Atomic_Ptr atomic; /* Implicitly initialized to NULL. --mbellon */
-   char *context;
-
-   context = Atomic_ReadPtr(&atomic);
-
-   if (context == NULL) {
-      void *p;
-      uint32 value = 0;
-
-      /*
-       * Use the machineID and executionID to hopefully start each machine
-       * and process/thread at a different place in the answer stream.
-       */
-
-      while (*machineID) {
-         value += *machineID++;
-      }
-
-      while (*executionID) {
-         value += *executionID++;
-      }
-
-      p = Random_QuickSeed(value);
-
-      if (Atomic_ReadIfEqualWritePtr(&atomic, NULL, p)) {
-         free(p);
-      }
-
-      context = Atomic_ReadPtr(&atomic);
-      ASSERT(context);
-   }
-
-   return (Random_Quick(context) >> 8) & 0xFFFF;
-}
-
-
 /*
  *-----------------------------------------------------------------------------
  *
@@ -1221,9 +1165,7 @@ MakeDirectory(ConstUnicode pathName)  // IN:
  */
 
 static int
-CreateEntryDirectory(const char *machineID,    // IN:
-                     const char *executionID,  // IN:
-                     ConstUnicode lockDir,     // IN:
+CreateEntryDirectory(ConstUnicode lockDir,     // IN:
                      Unicode *entryDirectory,  // OUT:
                      Unicode *entryFilePath,   // OUT:
                      Unicode *memberFilePath,  // OUT:
@@ -1295,7 +1237,7 @@ CreateEntryDirectory(const char *machineID,    // IN:
       }
 
       /* There is a small chance of collision/failure; grab stings now */
-      randomNumber = SimpleRandomNumber(machineID, executionID);
+      randomNumber = (FileSimpleRandom() >> 8) & 0xFFFF;
 
       *memberName = Unicode_Format("M%05u%s", randomNumber, FILELOCK_SUFFIX);
 
@@ -1545,9 +1487,7 @@ FileLockIntrinsic(ConstUnicode pathName,   // IN:
     * entry and member path names.
     */
 
-   *err = CreateEntryDirectory(myValues.machineID, myValues.executionID,
-                               lockDir,
-                               &entryDirectory, &entryFilePath,
+   *err = CreateEntryDirectory(lockDir, &entryDirectory, &entryFilePath,
                                &memberFilePath, &myValues.memberName);
 
    switch (*err) {
@@ -1803,9 +1743,7 @@ FileLockHackVMX(ConstUnicode pathName)  // IN:
    LOG(1, ("%s on %s (%s, %s).\n", __FUNCTION__, UTF8(pathName),
        myValues.machineID, myValues.executionID));
 
-   err = CreateEntryDirectory(myValues.machineID, myValues.executionID,
-                              lockDir,
-                              &entryDirectory, &entryFilePath,
+   err = CreateEntryDirectory(lockDir, &entryDirectory, &entryFilePath,
                               &memberFilePath, &myValues.memberName);
 
    if (err != 0) {