From: Oliver Kurth Date: Tue, 24 Oct 2017 21:07:33 +0000 (-0700) Subject: lib/file: Reduce the create directory spam X-Git-Tag: stable-10.3.0~229 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b88ff280ea0ea778ef9a4e422129bbf66291be73;p=thirdparty%2Fopen-vm-tools.git lib/file: Reduce the create directory spam The lib/file primitives (e.g. create file, create directory) may fail - but they also return errno/GetLastError. The caller should inspect a failure and decide what to do (or log). The lib/file primitives should not log. The client should do that. The primitives are the high performance path and failure isn't necessarily a failure, only the client can decide. The lib/file primitives (e.g. create file, create directory) may fail - but they also return errno/GetLastError. The caller should inspect a failure and decide what to do (or log). The lib/file primitives should not log. The client should do that. The primitives are the high performance path and failure isn't necessarily a failure, only the client can decide. --- diff --git a/open-vm-tools/lib/file/file.c b/open-vm-tools/lib/file/file.c index af39081d2..275614853 100644 --- a/open-vm-tools/lib/file/file.c +++ b/open-vm-tools/lib/file/file.c @@ -85,6 +85,8 @@ * uses the effective uid, but it's too risky to fix right now. * See PR 459242. * + * Errno/GetLastError is available upon failure. + * * Results: * TRUE file is accessible with the process' real uid * FALSE file doesn't exist or an error occured @@ -109,6 +111,8 @@ File_Exists(const char *pathName) // IN: May be NULL. * * If the given file exists, unlink it. * + * Errno/GetLastError is available upon failure. + * * Results: * Return 0 if the unlink is successful or if the file did not exist. * Otherwise return -1. @@ -169,6 +173,8 @@ File_SupportsMandatoryLock(const char *pathName) // IN: file to be locked * * Check if specified file is a directory or not. * + * Errno/GetLastError is available upon failure. + * * Results: * TRUE is a directory * FALSE is not a directory or an error occured @@ -196,6 +202,8 @@ File_IsDirectory(const char *pathName) // IN: * * Return the read / write / execute permissions of a file. * + * Errno/GetLastError is available upon failure. + * * Results: * TRUE if success, FALSE otherwise. * @@ -246,6 +254,8 @@ File_GetFilePermissions(const char *pathName, // IN: * followed. * WINDOWS: No symbolic links so no link following. * + * Errno/GetLastError is available upon failure. + * * Results: * Return 0 if the unlink is successful. Otherwise, returns -1. * @@ -271,6 +281,8 @@ File_Unlink(const char *pathName) // IN: * On Windows, there are no symbolic links so this is the same as * File_Unlink * + * Errno/GetLastError is available upon failure. + * * Results: * Return 0 if the unlink is successful. Otherwise, returns -1. * @@ -330,37 +342,6 @@ File_UnlinkRetry(const char *pathName, // IN: } -/* - *---------------------------------------------------------------------- - * - * FileCreateDirectoryEx -- - * - * Creates the specified directory with the specified permissions. - * - * Results: - * True if the directory is successfully created, false otherwise. - * - * Side effects: - * Creates the directory on disk. - * - *---------------------------------------------------------------------- - */ - -static int -FileCreateDirectoryEx(const char *pathName, // IN: - int mask) // IN: -{ - int err = FileCreateDirectory(pathName, mask); - - if (err != 0) { - Log(LGPFX" %s: Failed to create %s. Error = %d\n", - __FUNCTION__, pathName, err); - } - - return err; -} - - /* *---------------------------------------------------------------------- * @@ -368,8 +349,13 @@ FileCreateDirectoryEx(const char *pathName, // IN: * * Creates the specified directory with the specified permissions. * + * Errno/GetLastError is available upon failure. + * * Results: - * True if the directory is successfully created, false otherwise. + * TRUE Directory was created + * FALSE Directory creation failed. + * See File_EnsureDirectoryEx for dealing with directories that + * may exist. * * Side effects: * Creates the directory on disk. @@ -381,7 +367,7 @@ Bool File_CreateDirectoryEx(const char *pathName, // IN: int mask) // IN: { - int err = FileCreateDirectoryEx(pathName, mask); + int err = FileCreateDirectory(pathName, mask); return err == 0; } @@ -394,8 +380,13 @@ File_CreateDirectoryEx(const char *pathName, // IN: * * Creates the specified directory with 0777 permissions. * + * Errno/GetLastError is available upon failure. + * * Results: - * True if the directory is successfully created, false otherwise. + * TRUE Directory was created + * FALSE Directory creation failed. + * See File_EnsureDirectoryEx for dealing with directories that + * may exist. * * Side effects: * Creates the directory on disk. @@ -406,7 +397,9 @@ File_CreateDirectoryEx(const char *pathName, // IN: Bool File_CreateDirectory(const char *pathName) // IN: { - return File_CreateDirectoryEx(pathName, 0777); + int err = FileCreateDirectory(pathName, 0777); + + return err == 0; } @@ -418,6 +411,8 @@ File_CreateDirectory(const char *pathName) // IN: * If the directory doesn't exist, creates it. If the directory * already exists, do nothing and succeed. * + * Errno/GetLastError is available upon failure. + * * Results: * See above. * @@ -432,14 +427,8 @@ File_EnsureDirectoryEx(const char *pathName, // IN: int mask) // IN: { int err = FileCreateDirectory(pathName, mask); - Bool success = ((err == 0) || (err == EEXIST)); - - if (!success) { - Log(LGPFX" %s: Failed to create %s. Error = %d\n", - __FUNCTION__, pathName, err); - } - return success; + return ((err == 0) || (err == EEXIST)); } @@ -451,6 +440,8 @@ File_EnsureDirectoryEx(const char *pathName, // IN: * If the directory doesn't exist, creates it. If the directory * already exists, do nothing and succeed. * + * Errno/GetLastError is available upon failure. + * * Results: * See above. * @@ -1739,11 +1730,11 @@ File_CreateDirectoryHierarchyEx(const char *pathName, // IN: /* * If we check if the directory already exists and then we create it, - * there is a race between these two operations - that might cause this - * operation to fail with no reason. - * This is why we reverse the attempt and the check. + * there is a race between these two operations. Any failure can be + * confusing. We avoid this by attempting to create the directory before + * checking the type. */ - err = FileCreateDirectoryEx(temp, mask); + err = FileCreateDirectory(temp, mask); if (err == 0) { if (topmostCreated != NULL && *topmostCreated == NULL) { @@ -1769,6 +1760,11 @@ File_CreateDirectoryHierarchyEx(const char *pathName, // IN: } } + if (err != 0) { + Log(LGPFX" %s: Failure on '%s'. Error = %d\n", + __FUNCTION__, temp, err); + } + Posix_Free(temp); if (err != 0) {