From 8475ec068c213d0bf73f7686d82491a8f12e3b32 Mon Sep 17 00:00:00 2001 From: Reid Spencer Date: Thu, 29 Mar 2007 19:05:44 +0000 Subject: [PATCH] For PR789: Make the sys::Path::getFileStatus function more efficient by having it return a pointer to the FileStatus structure rather than copy it. Adjust uses of the function accordingly. Also, fix some memory issues in sys::Path. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@35476 91177308-0d34-0410-b5e6-96231b3b80d8 --- Makefile.config.in | 2 + Makefile.rules | 2 + include/llvm/System/Path.h | 24 +++++---- lib/Archive/Archive.cpp | 5 +- lib/Archive/ArchiveWriter.cpp | 5 +- lib/Bytecode/Archive/Archive.cpp | 5 +- lib/Bytecode/Archive/ArchiveWriter.cpp | 5 +- lib/Debugger/ProgramInfo.cpp | 7 +-- lib/Support/FileUtilities.cpp | 11 +++-- lib/System/Unix/Path.inc | 67 ++++++++++++++------------ lib/System/Unix/Signals.inc | 6 ++- lib/System/Win32/Path.inc | 28 +++++------ lib/System/Win32/Signals.inc | 6 ++- tools/llvm-ar/llvm-ar.cpp | 32 ++++++------ tools/llvm-db/Commands.cpp | 6 +-- tools/llvmc/CompilerDriver.cpp | 4 +- 16 files changed, 123 insertions(+), 92 deletions(-) diff --git a/Makefile.config.in b/Makefile.config.in index 699640de258..069b9724a34 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -74,6 +74,8 @@ PROJ_VERSION := 1.0 endif endif +LLVMMAKE := $(LLVM_SRC_ROOT)/make + PROJ_bindir := $(DESTDIR)$(PROJ_prefix)/bin PROJ_libdir := $(DESTDIR)$(PROJ_prefix)/lib PROJ_datadir := $(DESTDIR)$(PROJ_prefix)/share diff --git a/Makefile.rules b/Makefile.rules index 142b62e421e..13efb39ffa2 100644 --- a/Makefile.rules +++ b/Makefile.rules @@ -1760,3 +1760,5 @@ printvars:: $(Echo) "Module : " '$(Module)' $(Echo) "FilesToConfig: " '$(FilesToConfigPATH)' $(Echo) "SubDirs : " '$(SubDirs)' + $(Echo) "ProjLibsPaths: " '$(ProjLibsPaths)' + $(Echo) "ProjLibsOptions: " '$(ProjLibsOptions)' diff --git a/include/llvm/System/Path.h b/include/llvm/System/Path.h index 8d2f39107d4..84e6942cd42 100644 --- a/include/llvm/System/Path.h +++ b/include/llvm/System/Path.h @@ -166,6 +166,7 @@ namespace sys { /// @brief Construct an empty (and invalid) path. Path() : path(), status(0) {} ~Path() { delete status; } + Path(const Path &that) : path(that.path), status(0) {} /// This constructor will accept a std::string as a path. No checking is /// done on this path to determine if it is valid. To determine validity @@ -183,6 +184,9 @@ namespace sys { /// @brief Assignment Operator Path &operator=(const Path &that) { path = that.path; + if (status) + delete status; + status = 0; return *this; } @@ -223,9 +227,11 @@ namespace sys { /// @brief Determine if a path is syntactically valid or not. bool isValid() const; - /// This function determines if the contents of the path name are - /// empty. That is, the path has a zero length. This does NOT determine if - /// if the file is empty. Use the getSize method for that. + /// This function determines if the contents of the path name are empty. + /// That is, the path name has a zero length. This does NOT determine if + /// if the file is empty. To get the length of the file itself, Use the + /// getFileStatus() method and then the getSize() on the returned + /// FileStatus object /// @returns true iff the path is empty. /// @brief Determines if the path name is empty (invalid). bool isEmpty() const { return path.empty(); } @@ -357,13 +363,13 @@ namespace sys { /// This function returns status information about the file. The type of /// path (file or directory) is updated to reflect the actual contents - /// of the file system. This returns false on success, or true on error - /// and fills in the specified error string if specified. + /// of the file system. + /// @returns 0 on failure, with Error explaining why (if non-zero) + /// @returns a pointer to a FileStatus structure on success. /// @brief Get file status. - bool getFileStatus( - FileStatus &Status, ///< The resulting file status - bool forceUpdate = false, ///< Force an update from the file system - std::string *Error = 0 ///< Optional place to return an error msg. + const FileStatus *getFileStatus( + bool forceUpdate = false, ///< Force an update from the file system + std::string *Error = 0 ///< Optional place to return an error msg. ) const; /// @} diff --git a/lib/Archive/Archive.cpp b/lib/Archive/Archive.cpp index aff7ab8f1e6..f9fa80748bc 100644 --- a/lib/Archive/Archive.cpp +++ b/lib/Archive/Archive.cpp @@ -116,7 +116,10 @@ bool ArchiveMember::replaceWith(const sys::Path& newFile, std::string* ErrMsg) { path.getMagicNumber(magic,4); signature = magic.c_str(); std::string err; - if (path.getFileStatus(info, false, ErrMsg)) + const sys::FileStatus *FSinfo = path.getFileStatus(false, ErrMsg); + if (FSinfo) + info = *FSinfo; + else return true; } diff --git a/lib/Archive/ArchiveWriter.cpp b/lib/Archive/ArchiveWriter.cpp index 9d5b025b057..fc85374c020 100644 --- a/lib/Archive/ArchiveWriter.cpp +++ b/lib/Archive/ArchiveWriter.cpp @@ -163,7 +163,10 @@ Archive::addFileBefore(const sys::Path& filePath, iterator where, mbr->data = 0; mbr->path = filePath; - if (mbr->path.getFileStatus(mbr->info, false, ErrMsg)) + const sys::FileStatus *FSInfo = mbr->path.getFileStatus(false, ErrMsg); + if (FSInfo) + mbr->info = *FSInfo; + else return true; unsigned flags = 0; diff --git a/lib/Bytecode/Archive/Archive.cpp b/lib/Bytecode/Archive/Archive.cpp index aff7ab8f1e6..f9fa80748bc 100644 --- a/lib/Bytecode/Archive/Archive.cpp +++ b/lib/Bytecode/Archive/Archive.cpp @@ -116,7 +116,10 @@ bool ArchiveMember::replaceWith(const sys::Path& newFile, std::string* ErrMsg) { path.getMagicNumber(magic,4); signature = magic.c_str(); std::string err; - if (path.getFileStatus(info, false, ErrMsg)) + const sys::FileStatus *FSinfo = path.getFileStatus(false, ErrMsg); + if (FSinfo) + info = *FSinfo; + else return true; } diff --git a/lib/Bytecode/Archive/ArchiveWriter.cpp b/lib/Bytecode/Archive/ArchiveWriter.cpp index 9d5b025b057..fc85374c020 100644 --- a/lib/Bytecode/Archive/ArchiveWriter.cpp +++ b/lib/Bytecode/Archive/ArchiveWriter.cpp @@ -163,7 +163,10 @@ Archive::addFileBefore(const sys::Path& filePath, iterator where, mbr->data = 0; mbr->path = filePath; - if (mbr->path.getFileStatus(mbr->info, false, ErrMsg)) + const sys::FileStatus *FSInfo = mbr->path.getFileStatus(false, ErrMsg); + if (FSInfo) + mbr->info = *FSInfo; + else return true; unsigned flags = 0; diff --git a/lib/Debugger/ProgramInfo.cpp b/lib/Debugger/ProgramInfo.cpp index a315a2d5ba2..d811f6075be 100644 --- a/lib/Debugger/ProgramInfo.cpp +++ b/lib/Debugger/ProgramInfo.cpp @@ -194,9 +194,10 @@ void SourceFunctionInfo::getSourceLocation(unsigned &RetLineNo, ProgramInfo::ProgramInfo(Module *m) : M(m), ProgramTimeStamp(0,0) { assert(M && "Cannot create program information with a null module!"); - sys::FileStatus Stat; - if (!sys::Path(M->getModuleIdentifier()).getFileStatus(Stat)) - ProgramTimeStamp = Stat.getTimestamp(); + const sys::FileStatus *Stat; + Stat = sys::Path(M->getModuleIdentifier()).getFileStatus(); + if (Stat) + ProgramTimeStamp = Stat->getTimestamp(); SourceFilesIsComplete = false; SourceFunctionsIsComplete = false; diff --git a/lib/Support/FileUtilities.cpp b/lib/Support/FileUtilities.cpp index a0cdf09deec..1ea5ddada9e 100644 --- a/lib/Support/FileUtilities.cpp +++ b/lib/Support/FileUtilities.cpp @@ -152,16 +152,17 @@ int llvm::DiffFilesWithTolerance(const sys::Path &FileA, const sys::Path &FileB, double AbsTol, double RelTol, std::string *Error) { - sys::FileStatus FileAStat, FileBStat; - if (FileA.getFileStatus(FileAStat, false, Error)) + const sys::FileStatus *FileAStat = FileA.getFileStatus(false, Error); + if (!FileAStat) return 2; - if (FileB.getFileStatus(FileBStat, false, Error)) + const sys::FileStatus *FileBStat = FileB.getFileStatus(false, Error); + if (!FileBStat) return 2; // Check for zero length files because some systems croak when you try to // mmap an empty file. - size_t A_size = FileAStat.getSize(); - size_t B_size = FileBStat.getSize(); + size_t A_size = FileAStat->getSize(); + size_t B_size = FileBStat->getSize(); // If they are both zero sized then they're the same if (A_size == 0 && B_size == 0) diff --git a/lib/System/Unix/Path.inc b/lib/System/Unix/Path.inc index 5557282964e..6ceffcdfe31 100644 --- a/lib/System/Unix/Path.inc +++ b/lib/System/Unix/Path.inc @@ -333,9 +333,10 @@ bool Path::canExecute() const { if (0 != access(path.c_str(), R_OK | X_OK )) return false; - struct stat st; - int r = stat(path.c_str(), &st); - if (r != 0 || !S_ISREG(st.st_mode)) + if (const FileStatus *fs = getFileStatus(true, 0)) { + if (!S_ISREG(fs->mode)) + return false; + } else return false; return true; } @@ -362,12 +363,14 @@ Path::getLast() const { return path.substr(pos+1); } -bool -Path::getFileStatus(FileStatus &info, bool update, std::string *ErrStr) const { +const FileStatus* +Path::getFileStatus(bool update, std::string *ErrStr) const { if (status == 0 || update) { struct stat buf; - if (0 != stat(path.c_str(), &buf)) - return MakeErrMsg(ErrStr, path + ": can't get status of file"); + if (0 != stat(path.c_str(), &buf)) { + MakeErrMsg(ErrStr, path + ": can't get status of file"); + return 0; + } if (status == 0) status = new FileStatus; status->fileSize = buf.st_size; @@ -379,8 +382,7 @@ Path::getFileStatus(FileStatus &info, bool update, std::string *ErrStr) const { status->isDir = S_ISDIR(buf.st_mode); status->isFile = S_ISREG(buf.st_mode); } - info = *status; - return false; + return status; } static bool AddPermissionBits(const Path &File, int bits) { @@ -392,12 +394,12 @@ static bool AddPermissionBits(const Path &File, int bits) { umask(mask); // Restore the umask. // Get the file's current mode. - FileStatus Stat; - if (File.getFileStatus(Stat)) return false; - - // Change the file to have whichever permissions bits from 'bits' - // that the umask would not disable. - if ((chmod(File.c_str(), (Stat.getMode() | (bits & ~mask)))) == -1) + if (const FileStatus *fs = File.getFileStatus()) { + // Change the file to have whichever permissions bits from 'bits' + // that the umask would not disable. + if ((chmod(File.c_str(), (fs->getMode() | (bits & ~mask)))) == -1) + return false; + } else return false; return true; @@ -593,24 +595,25 @@ Path::createTemporaryFileOnDisk(bool reuse_current, std::string* ErrMsg) { bool Path::eraseFromDisk(bool remove_contents, std::string *ErrStr) const { FileStatus Status; - if (getFileStatus(Status, ErrStr)) - return true; + if (const FileStatus *Status = getFileStatus(false, ErrStr)) { + // Note: this check catches strange situations. In all cases, LLVM should + // only be involved in the creation and deletion of regular files. This + // check ensures that what we're trying to erase is a regular file. It + // effectively prevents LLVM from erasing things like /dev/null, any block + // special file, or other things that aren't "regular" files. + if (Status->isFile) { + if (unlink(path.c_str()) != 0) + return MakeErrMsg(ErrStr, path + ": can't destroy file"); + return false; + } - // Note: this check catches strange situations. In all cases, LLVM should only - // be involved in the creation and deletion of regular files. This check - // ensures that what we're trying to erase is a regular file. It effectively - // prevents LLVM from erasing things like /dev/null, any block special file, - // or other things that aren't "regular" files. - if (Status.isFile) { - if (unlink(path.c_str()) != 0) - return MakeErrMsg(ErrStr, path + ": can't destroy file"); - return false; - } - - if (!Status.isDir) { - if (ErrStr) *ErrStr = "not a file or directory"; + if (!Status->isDir) { + if (ErrStr) *ErrStr = "not a file or directory"; + return true; + } + } else return true; - } + if (remove_contents) { // Recursively descend the directory to remove its contents. std::string cmd = "/bin/rm -rf " + path; @@ -629,7 +632,7 @@ Path::eraseFromDisk(bool remove_contents, std::string *ErrStr) const { if (rmdir(pathname) != 0) return MakeErrMsg(ErrStr, - std::string(pathname) + ": can't destroy directory"); + std::string(pathname) + ": can't erase directory"); return false; } diff --git a/lib/System/Unix/Signals.inc b/lib/System/Unix/Signals.inc index 35c628ba33f..a471b95a999 100644 --- a/lib/System/Unix/Signals.inc +++ b/lib/System/Unix/Signals.inc @@ -168,8 +168,10 @@ bool sys::RemoveFileOnSignal(const sys::Path &Filename, std::string* ErrMsg) { // RemoveDirectoryOnSignal - The public API bool sys::RemoveDirectoryOnSignal(const sys::Path& path, std::string* ErrMsg) { // Not a directory? - sys::FileStatus Status; - if (path.getFileStatus(Status) || !Status.isDir) { + const sys::FileStatus *Status = path.getFileStatus(false, ErrMsg); + if (!Status) + return true; + if (!Status->isDir) { if (ErrMsg) *ErrMsg = path.toString() + " is not a directory"; return true; diff --git a/lib/System/Win32/Path.inc b/lib/System/Win32/Path.inc index 5bb2e39e949..57d75358b81 100644 --- a/lib/System/Win32/Path.inc +++ b/lib/System/Win32/Path.inc @@ -306,13 +306,15 @@ Path::getLast() const { return path.substr(pos+1); } -bool -Path::getFileStatus(FileStatus &info, bool update, std::string *ErrStr) const { +const FileStatus * +Path::getFileStatus(bool update, std::string *ErrStr) const { if (status == 0 || update) { WIN32_FILE_ATTRIBUTE_DATA fi; - if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &fi)) - return MakeErrMsg(ErrStr, "getStatusInfo():" + std::string(path) + + if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &fi)) { + MakeErrMsg(ErrStr, "getStatusInfo():" + std::string(path) + ": Can't get status: "); + return 0; + } if (status == 0) status = new FileStatus; @@ -337,8 +339,7 @@ Path::getFileStatus(FileStatus &info, bool update, std::string *ErrStr) const { status->isDir = fi.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY; } - info = *status; - return false; + return status; } bool Path::makeReadableOnDisk(std::string* ErrMsg) { @@ -369,11 +370,10 @@ bool Path::makeExecutableOnDisk(std::string* ErrMsg) { bool Path::getDirectoryContents(std::set& result, std::string* ErrMsg) const { - FileStatus Status; - if (getFileStatus(Status, ErrMsg)) + const FileStatus *Status = getFileStatus(false, ErrMsg); + if (!Status) return true; - - if (!Status.isDir) { + if (!Status->isDir) { MakeErrMsg(ErrMsg, path + ": not a directory"); return true; } @@ -567,11 +567,11 @@ Path::createFileOnDisk(std::string* ErrMsg) { bool Path::eraseFromDisk(bool remove_contents, std::string *ErrStr) const { - FileStatus Status; - if (getFileStatus(Status, ErrStr)) + const FileStatus *Status = getFileStatus(false, ErrStr); + if (!Status) return false; - if (Status.isFile) { + if (Status->isFile) { DWORD attr = GetFileAttributes(path.c_str()); // If it doesn't exist, we're done. @@ -588,7 +588,7 @@ Path::eraseFromDisk(bool remove_contents, std::string *ErrStr) const { if (!DeleteFile(path.c_str())) return MakeErrMsg(ErrStr, path + ": Can't destroy file: "); return false; - } else if (Status.isDir) { + } else if (Status->isDir) { // If it doesn't exist, we're done. if (!exists()) return false; diff --git a/lib/System/Win32/Signals.inc b/lib/System/Win32/Signals.inc index 0c93b228c3c..8adf7674faf 100644 --- a/lib/System/Win32/Signals.inc +++ b/lib/System/Win32/Signals.inc @@ -101,8 +101,10 @@ bool sys::RemoveFileOnSignal(const sys::Path &Filename, std::string* ErrMsg) { // RemoveDirectoryOnSignal - The public API bool sys::RemoveDirectoryOnSignal(const sys::Path& path, std::string* ErrMsg) { // Not a directory? - sys::FileStatus Status; - if (path.getFileStatus(Status) || !Status.isDir) { + const sys::FileStatus *Status = path.getFileStatus(false, ErrMsg); + if (!Status) + return true; + if (!Status->isDir) { if (ErrMsg) *ErrMsg = path.toString() + " is not a directory"; return true; diff --git a/tools/llvm-ar/llvm-ar.cpp b/tools/llvm-ar/llvm-ar.cpp index ea3a59b381d..649eb7a7be7 100644 --- a/tools/llvm-ar/llvm-ar.cpp +++ b/tools/llvm-ar/llvm-ar.cpp @@ -281,16 +281,16 @@ recurseDirectories(const sys::Path& path, for (std::set::iterator I = content.begin(), E = content.end(); I != E; ++I) { // Make sure it exists and is a directory - sys::FileStatus Status; - if (I->getFileStatus(Status)) { - if (Status.isDir) { - std::set moreResults; - if (recurseDirectories(*I, moreResults, ErrMsg)) - return true; - result.insert(moreResults.begin(), moreResults.end()); - } else { + const sys::FileStatus *Status = I->getFileStatus(false, ErrMsg); + if (!Status) + return true; + if (Status->isDir) { + std::set moreResults; + if (recurseDirectories(*I, moreResults, ErrMsg)) + return true; + result.insert(moreResults.begin(), moreResults.end()); + } else { result.insert(*I); - } } } } @@ -308,11 +308,11 @@ bool buildPaths(bool checkExistence, std::string* ErrMsg) { if (checkExistence) { if (!aPath.exists()) throw std::string("File does not exist: ") + Members[i]; - sys::FileStatus si; std::string Err; - if (aPath.getFileStatus(si, false, &Err)) + const sys::FileStatus *si = aPath.getFileStatus(false, &Err); + if (!si) throw Err; - if (si.isDir) { + if (si->isDir) { std::set dirpaths; if (recurseDirectories(aPath, dirpaths, ErrMsg)) return true; @@ -644,14 +644,14 @@ doReplaceOrInsert(std::string* ErrMsg) { } if (found != remaining.end()) { - sys::FileStatus si; std::string Err; - if (found->getFileStatus(si, false, &Err)) + const sys::FileStatus *si = found->getFileStatus(false, &Err); + if (!si) return true; - if (si.isDir) { + if (si->isDir) { if (OnlyUpdate) { // Replace the item only if it is newer. - if (si.modTime > I->getModTime()) + if (si->modTime > I->getModTime()) if (I->replaceWith(*found, ErrMsg)) return true; } else { diff --git a/tools/llvm-db/Commands.cpp b/tools/llvm-db/Commands.cpp index da07769fbb0..5a8aa2f132f 100644 --- a/tools/llvm-db/Commands.cpp +++ b/tools/llvm-db/Commands.cpp @@ -50,11 +50,11 @@ void CLIDebugger::startProgramRunning() { // If the program has been modified, reload it! sys::Path Program(Dbg.getProgramPath()); - sys::FileStatus Status; std::string Err; - if (Program.getFileStatus(Status, &Err)) + const sys::FileStatus *Status = Program.getFileStatus(false, &Err); + if (!Status) throw Err; - if (TheProgramInfo->getProgramTimeStamp() != Status.getTimestamp()) { + if (TheProgramInfo->getProgramTimeStamp() != Status->getTimestamp()) { std::cout << "'" << Program << "' has changed; re-reading program.\n"; // Unload an existing program. This kills the program if necessary. diff --git a/tools/llvmc/CompilerDriver.cpp b/tools/llvmc/CompilerDriver.cpp index c153df8ac48..3d91ad96765 100644 --- a/tools/llvmc/CompilerDriver.cpp +++ b/tools/llvmc/CompilerDriver.cpp @@ -195,8 +195,8 @@ private: void cleanup() { if (!isSet(KEEP_TEMPS_FLAG)) { - sys::FileStatus Status; - if (!TempDir.getFileStatus(Status) && Status.isDir) + const sys::FileStatus *Status = TempDir.getFileStatus(); + if (Status && Status->isDir) TempDir.eraseFromDisk(/*remove_contents=*/true); } else { std::cout << "Temporary files are in " << TempDir << "\n"; -- 2.34.1