From 3be660995497ea5f141722ba745c8fbf1d72164d Mon Sep 17 00:00:00 2001 From: Bill Chapman Date: Fri, 2 Aug 2024 11:13:12 -0400 Subject: [PATCH] Bdls filesystemutil remove socket drqs 176123156 (#4850) --- groups/bdl/bdls/bdls_filesystemutil.cpp | 124 +++++----------------- groups/bdl/bdls/bdls_filesystemutil.t.cpp | 89 ++++++++++++++-- 2 files changed, 102 insertions(+), 111 deletions(-) diff --git a/groups/bdl/bdls/bdls_filesystemutil.cpp b/groups/bdl/bdls/bdls_filesystemutil.cpp index 25f2299eaf..4e464d8af3 100644 --- a/groups/bdl/bdls/bdls_filesystemutil.cpp +++ b/groups/bdl/bdls/bdls_filesystemutil.cpp @@ -1003,58 +1003,23 @@ int u_removeContentsOfTree( continue; } - bool isDir = false; FileDescriptor entryFD = ::openat(dirFD, entry.d_name, - O_RDONLY | O_NOFOLLOW | O_NONBLOCK, + O_DIRECTORY | O_RDONLY | O_NOFOLLOW, 0); - if (-1 == entryFD) { - if (ENOENT == errno) { - // Another thread or process has deleted the file. + if (-1 != entryFD) { + // 'entry.d_name' is a directory. - return -3; // RETURN - } -#ifdef BSLS_PLATFORM_OS_AIX - // On AIX, '::openat' fails on pipes, but'::faccessat' is able to - // confirm that they exist. - - if (EINVAL == errno && 0 == ::faccessat(dirFD, - entry.d_name, - F_OK, - AT_SYMLINK_NOFOLLOW)) { - // It's a pipe -- delete it like a plain file. - - ; - } - else -#endif - if (ELOOP != errno) { - // not a symlink -- error - - return -4; // RETURN - } - - // it's a symlink or pipe -- we'll just delete it like a plain file - } - else { - StatResult statResult; - if (0 != ::performFStat(entryFD, &statResult)) { - return 0 == ::close(entryFD) ? -5 : -6; // RETURN - } - isDir = S_ISDIR(statResult.st_mode); - if (isDir) { - // 'u_removeContentsOfTree' will close 'entryFD'. + // 'u_removeContentsOfTree' will close 'entryFD'. - const int rc = u_removeContentsOfTree(entryFD); - if (0 != rc) { - return -7; // RETURN - } - } else if (0 != ::close(entryFD)) { - return -8; // RETURN + const int rc = u_removeContentsOfTree(entryFD); + if (0 != rc) { + return -7; // RETURN } } - if (0 != ::unlinkat(dirFD, entry.d_name, isDir ? AT_REMOVEDIR : 0)) { + const int flag = -1 != entryFD ? AT_REMOVEDIR : 0; + if (0 != ::unlinkat(dirFD, entry.d_name, flag)) { return -9; // RETURN } @@ -2561,84 +2526,43 @@ int FilesystemUtil::remove(const char *path, bool recursiveFlag) 0, &invokeCloseFD); - bool isDir = false; - bool leafFDIsOpen = false; - bool isError = false; - // If 'leaf' is a symlink, we just want to delete the symlink, not what it // points to, hence 'O_NOFOLLOW'. FileDescriptor leafFD = ::openat(parentFD, leaf.c_str(), - O_RDONLY | O_NOFOLLOW | O_NONBLOCK, + O_DIRECTORY | O_RDONLY | O_NOFOLLOW, 0); if (-1 == leafFD) { -#ifdef BSLS_PLATFORM_OS_AIX - // On AIX, '::openat' fails on pipes, but'::faccessat' is able to - // confirm that they exist. - - if (EINVAL == errno && 0 == ::faccessat(parentFD, - leaf.c_str(), - F_OK, - AT_SYMLINK_NOFOLLOW)) { - // It's a pipe -- delete it like a plain file. - - ; - } - else -#endif - if (ELOOP != errno) { - // not a symlink -- error - - return -6; // RETURN - } - - // it's a symlink or pipe -- we'll just delete it like a plain file + // It's a plain file, symlink, pipe, or socket -- we'll just delete it + // like a plain file. if (isErrorIfNotDir) { - // At this point, the file is either a symlink or (on AIX) maybe a - // pipe, and we treat those like plain files. 'isErrorIfNotDir' - // indicates that 'path' ended with '/' or "/.", which indicate - // that the client thought it was a directory, which is - // inconsistent and an error. + // 'isErrorIfNotDir' indicates that 'path' ended with '/' or "/.", + // which indicate that the client thought it was a directory, which + // is inconsistent and an error. return -7; // RETURN } } else { - leafFDIsOpen = true; + // 'leaf' is a directory. - StatResult statResult; - if (0 != ::performFStat(leafFD, &statResult)) { - return 0 != ThisUtil::close(leafFD) ? -8 : -9; // RETURN - } - isDir = S_ISDIR(statResult.st_mode); - if (!isDir) { - isError = isErrorIfNotDir; - } - else if (recursiveFlag) { + if (recursiveFlag) { const int rc = u_removeContentsOfTree(leafFD); if (0 != rc) { - return -10; // RETURN + return -100 + rc; // RETURN } - // 'u_removeContentsOfTree' has closed 'leafFD'. Note that it - // removed files in the directory 'leaf', but not the directory - // 'leaf' itself. - - leafFDIsOpen = false; + // 'u_removeContentsOfTree' closed 'leafFD' + } + else if (0 != ThisUtil::close(leafFD)) { + return -8; // RETURN } } - if (leafFDIsOpen && 0 != ThisUtil::close(leafFD)) { - return -11; // RETURN - } - - if (isError) { - return -12; // RETURN - } - - return ::unlinkat(parentFD, leaf.c_str(), isDir ? AT_REMOVEDIR : 0) + const int flag = -1 != leafFD ? AT_REMOVEDIR : 0; + return ::unlinkat(parentFD, leaf.c_str(), flag) ? -13 : 0; } diff --git a/groups/bdl/bdls/bdls_filesystemutil.t.cpp b/groups/bdl/bdls/bdls_filesystemutil.t.cpp index 69d8df7cac..61644d1ca0 100644 --- a/groups/bdl/bdls/bdls_filesystemutil.t.cpp +++ b/groups/bdl/bdls/bdls_filesystemutil.t.cpp @@ -128,14 +128,14 @@ using bsls::NameOf; // [26] int mapChecked(FileDescriptor, void **, Offset, bsl::size_t, int); // [26] int unmap(void *, bsl::size_t); // [28] int getLastModificationTime(bdlt::Datetime *, FileDescriptor); -// [29] bool isSymbolicLink(STRING_TYPE); -// [29] int getSymbolicLinkTarget(STRING_TYPE *, STRING_TYPE); -// [30] createTemporarySubdirectory(bsl::string*, const bsl::string_view&) -// [30] createTemporarySubdirectory(std::string*, const bsl::string_view&) -// [30] createTemporarySubdirectory(pmr::string*, const bsl::string_view&) -// [31] int remove(const char *); +// [29] createTemporarySubdirectory(bsl::string*, const bsl::string_view&) +// [29] createTemporarySubdirectory(std::string*, const bsl::string_view&) +// [29] createTemporarySubdirectory(pmr::string*, const bsl::string_view&) +// [30] int remove(const char *); +// [30] int remove(STRING_TYPE); // [31] int remove(STRING_TYPE); -// [32] int remove(STRING_TYPE); +// [32] bool isSymbolicLink(STRING_TYPE); +// [32] int getSymbolicLinkTarget(STRING_TYPE *, STRING_TYPE); // // FREE OPERATORS // [27] ostream& operator<<(ostream&, Whence); @@ -154,8 +154,9 @@ using bsls::NameOf; // [20] CONCERN: directory permissions // [21] CONCERN: error codes for 'createDirectories' // [21] CONCERN: error codes for 'createPrivateDirectory' -// [34] TESTING USAGE EXAMPLE 2 -// [33] TESTING USAGE EXAMPLE 1 +// [33] TESTING REMOVE UNIX SOCKET +// [35] TESTING USAGE EXAMPLE 2 +// [34] TESTING USAGE EXAMPLE 1 // ============================================================================ // STANDARD BDE ASSERT TEST FUNCTION @@ -4440,7 +4441,7 @@ int main(int argc, char *argv[]) ASSERT(0 == Obj::setWorkingDirectory(tmpWorkingDir)); switch(test) { case 0: - case 34: { + case 35: { // -------------------------------------------------------------------- // TESTING USAGE EXAMPLE 2 // @@ -4525,7 +4526,7 @@ int main(int argc, char *argv[]) ASSERT(0 == bdls::PathUtil::popLeaf(&logPath)); ASSERT(0 == Obj::remove(logPath.c_str(), true)); } break; - case 33: { + case 34: { // -------------------------------------------------------------------- // TESTING USAGE EXAMPLE 1 // @@ -4644,6 +4645,72 @@ int main(int argc, char *argv[]) ASSERT(0 == bdls::PathUtil::popLeaf(&logPath)); ASSERT(0 == Obj::remove(logPath.c_str(), true)); } break; + case 33: { + // -------------------------------------------------------------------- + // TESTING REMOVE UNIX SOCKET (DRQS 176123156) + // + // Concern: + //: 1 That 'Obj::remove' can remove an open socket. + // + // Plan: + //: 1 Open a socket between and bind it to the file system. + //: + //: 2 Remove the file manifestation of the socket. + //: + //: 3 Iterate twice, in the second iteration close the socket file + //: descriptor before removing the file. + // + // Testing: + // TESTING REMOVE UNIX SOCKET + // -------------------------------------------------------------------- + +#ifndef BSLS_PLATFORM_OS_UNIX + if (verbose) cout << + "TESTING REMOVE UNIX SOCKET -- skipped on Windows\n" + "================================================\n"; +#else + if (verbose) cout << "TESTING REMOVE UNIX SOCKET (DRQS 176123156)\n" + "==============-----------------============\n"; + + ::sockaddr_un jointAddrRaw; + ::sockaddr *jointAddr_p = + reinterpret_cast< ::sockaddr *>(&jointAddrRaw); + char *socketName = jointAddrRaw.sun_path; + + for (int closeFirst = 0; closeFirst < 2; ++closeFirst) { + const char *fileName = closeFirst ? "closedFile" : "openFile"; + ASSERT(bsl::strlen(fileName) < sizeof(jointAddrRaw.sun_path)); + + ::memset(jointAddr_p, 0, sizeof(jointAddrRaw)); + jointAddrRaw.sun_family = AF_UNIX; + bsl::strcpy(socketName, fileName); + + int sfd = ::socket(AF_UNIX, SOCK_STREAM, 0); + ASSERT(0 <= sfd); + + Obj::remove(socketName); + int rc = ::bind(sfd, jointAddr_p, sizeof(jointAddrRaw)); + ASSERT(0 == rc); + + if (closeFirst) { + ASSERT(0 == ::close(sfd)); + } + + if (veryVerbose) { + char lsBuf[100] = { "ls -l " }; + bsl::strcat(lsBuf, socketName); + ASSERTV(lsBuf, 0 == ::system(lsBuf)); + } + + ASSERT(Obj::exists(socketName)); + + rc = Obj::remove(socketName); + ASSERTV(closeFirst, rc, errno, ::strerror(errno), 0 == rc); + + ASSERT(! Obj::exists(socketName)); + } +#endif + } break; case 32: { // -------------------------------------------------------------------- // TESTING SYMLINKS