From: VMware, Inc <> Date: Mon, 26 Apr 2010 18:23:57 +0000 (-0700) Subject: lib/file: pick up some review comments X-Git-Tag: 2010.04.25-253928~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bc8c77febf3f2d3290b8beb33ca15b5210f8bf85;p=thirdparty%2Fopen-vm-tools.git 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 --- diff --git a/open-vm-tools/lib/file/file.c b/open-vm-tools/lib/file/file.c index 8d711bb34..6cffaa7e5 100644 --- 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 21b962792..dfdfc10be 100644 --- 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 4736cd53d..f805274b1 100644 --- 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) {