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

@@ -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

Makefile.rules

@@ -1760,3 +1760,5 @@ printvars::
 	$(Echo) "Module       : " '$(Module)'
 	$(Echo) "FilesToConfig: " '$(FilesToConfigPATH)'
 	$(Echo) "SubDirs      : " '$(SubDirs)'
+	$(Echo) "ProjLibsPaths: " '$(ProjLibsPaths)'
+	$(Echo) "ProjLibsOptions: " '$(ProjLibsOptions)'

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;
 
     /// @}

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;
   }
 

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;

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;
   }
 

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;

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;

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)

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;
 }
 

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;

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<Path>& 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;

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;

tools/llvm-ar/llvm-ar.cpp

@@ -281,16 +281,16 @@ recurseDirectories(const sys::Path& path,
     for (std::set<sys::Path>::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<sys::Path> 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<sys::Path> 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<sys::Path> 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 {

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.

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";