change the 'is directory' indicator to be a null-or-not

pointer that is passed down through the APIs, and make
FileSystemStatCache::get be the one that filters out
directory lookups that hit files.  This also paves the
way to have stat queries be able to return opened files.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@120060 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 75 insertions, 35 deletions

diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h
index 8e97062b44..a32f65a598 100644
--- a/include/clang/Basic/FileManager.h
+++ b/include/clang/Basic/FileManager.h
@@ -131,7 +131,8 @@ class FileManager {
   // Caching.
   llvm::OwningPtr<FileSystemStatCache> StatCache;
 
-  bool getStatValue(const char *Path, struct stat &StatBuf, bool isForDir);
+  bool getStatValue(const char *Path, struct stat &StatBuf,
+                    int *FileDescriptor);
 public:
   FileManager(const FileSystemOptions &FileSystemOpts);
   ~FileManager();
diff --git a/include/clang/Basic/FileSystemStatCache.h b/include/clang/Basic/FileSystemStatCache.h
index e9da8cf15c..77828b3ecb 100644
--- a/include/clang/Basic/FileSystemStatCache.h
+++ b/include/clang/Basic/FileSystemStatCache.h
@@ -39,13 +39,15 @@ public:
   /// FileSystemStatCache::get - Get the 'stat' information for the specified
   /// path, using the cache to accellerate it if possible.  This returns true if
   /// the path does not exist or false if it exists.
-  static bool get(const char *Path, struct stat &StatBuf,
-                  FileSystemStatCache *Cache) {
-    if (Cache)
-      return Cache->getStat(Path, StatBuf) == CacheMissing;
-    
-    return ::stat(Path, &StatBuf) != 0;
-  }
+  ///
+  /// If FileDescriptor is non-null, then this lookup should only return success
+  /// for files (not directories).  If it is null this lookup should only return
+  /// success for directories (not files).  On a successful file lookup, the
+  /// implementation can optionally fill in FileDescriptor with a valid
+  /// descriptor and the client guarantees that it will close it.
+  static bool get(const char *Path, struct stat &StatBuf, int *FileDescriptor,
+                  FileSystemStatCache *Cache);
+  
   
   /// \brief Sets the next stat call cache in the chain of stat caches.
   /// Takes ownership of the given stat cache.
@@ -62,15 +64,17 @@ public:
   FileSystemStatCache *takeNextStatCache() { return NextStatCache.take(); }
   
 protected:
-  virtual LookupResult getStat(const char *Path, struct stat &StatBuf) = 0;
+  virtual LookupResult getStat(const char *Path, struct stat &StatBuf,
+                               int *FileDescriptor) = 0;
 
-  LookupResult statChained(const char *Path, struct stat &StatBuf) {
+  LookupResult statChained(const char *Path, struct stat &StatBuf,
+                           int *FileDescriptor) {
     if (FileSystemStatCache *Next = getNextStatCache())
-      return Next->getStat(Path, StatBuf);
+      return Next->getStat(Path, StatBuf, FileDescriptor);
     
     // If we hit the end of the list of stat caches to try, just compute and
     // return it without a cache.
-    return get(Path, StatBuf, 0) ? CacheMissing : CacheExists;
+    return get(Path, StatBuf, FileDescriptor, 0) ? CacheMissing : CacheExists;
   }
 };
 
@@ -88,7 +92,8 @@ public:
   iterator begin() const { return StatCalls.begin(); }
   iterator end() const { return StatCalls.end(); }
   
-  virtual LookupResult getStat(const char *Path, struct stat &StatBuf);
+  virtual LookupResult getStat(const char *Path, struct stat &StatBuf,
+                               int *FileDescriptor);
 };
 
 } // end namespace clang
diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
index b56d96f21f..27a4dfe544 100644
--- a/lib/Basic/FileManager.cpp
+++ b/lib/Basic/FileManager.cpp
@@ -33,10 +33,6 @@ using namespace clang;
 // FIXME: Enhance libsystem to support inode and other fields.
 #include <sys/stat.h>
 
-#if defined(_MSC_VER)
-#define S_ISDIR(s) (_S_IFDIR & s)
-#endif
-
 /// NON_EXISTENT_DIR - A special value distinct from null that is used to
 /// represent a dir name that doesn't exist on the disk.
 #define NON_EXISTENT_DIR reinterpret_cast<DirectoryEntry*>((intptr_t)-1)
@@ -248,7 +244,7 @@ const DirectoryEntry *FileManager::getDirectory(llvm::StringRef Filename) {
 
   // Check to see if the directory exists.
   struct stat StatBuf;
-  if (getStatValue(InterndDirName, StatBuf, true))
+  if (getStatValue(InterndDirName, StatBuf, 0/*directory lookup*/))
     return 0;
 
   // It exists.  See if we have already opened a directory with the same inode.
@@ -304,8 +300,9 @@ const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
   // FIXME: This will reduce the # syscalls.
 
   // Nope, there isn't.  Check to see if the file exists.
+  int FileDescriptor = -1;
   struct stat StatBuf;
-  if (getStatValue(InterndFileName, StatBuf, false))
+  if (getStatValue(InterndFileName, StatBuf, &FileDescriptor))
     return 0;
 
   // It exists.  See if we have already opened a file with the same inode.
@@ -313,8 +310,13 @@ const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
   FileEntry &UFE = UniqueFiles.getFile(InterndFileName, StatBuf);
 
   NamedFileEnt.setValue(&UFE);
-  if (UFE.getName())  // Already have an entry with this inode, return it.
+  if (UFE.getName()) { // Already have an entry with this inode, return it.
+    // If the stat process opened the file, close it to avoid a FD leak.
+    if (FileDescriptor != -1)
+      close(FileDescriptor);
+    
     return &UFE;
+  }
 
   // Otherwise, we don't have this directory yet, add it.
   // FIXME: Change the name to be a char* that points back to the 'FileEntries'
@@ -324,6 +326,7 @@ const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
   UFE.ModTime = StatBuf.st_mtime;
   UFE.Dir     = DirInfo;
   UFE.UID     = NextFileUID++;
+  UFE.FD      = FileDescriptor;
   return &UFE;
 }
 
@@ -366,10 +369,12 @@ FileManager::getVirtualFile(llvm::StringRef Filename, off_t Size,
   
   // If this virtual file resolves to a file, also map that file to the 
   // newly-created file entry.
+  int FileDescriptor = -1;
   struct stat StatBuf;
-  if (getStatValue(InterndFileName, StatBuf, false))
+  if (getStatValue(InterndFileName, StatBuf, &FileDescriptor))
     return UFE;
-    
+  
+  UFE->FD = FileDescriptor;
   llvm::sys::Path FilePath(UFE->Name);
   FilePath.makeAbsolute();
   FileEntries[FilePath.str()] = UFE;
@@ -414,18 +419,18 @@ getBufferForFile(llvm::StringRef Filename, std::string *ErrorStr) {
 /// The isForDir member indicates whether this is a directory lookup or not.
 /// This will return failure if the lookup isn't the expected kind.
 bool FileManager::getStatValue(const char *Path, struct stat &StatBuf,
-                               bool isForDir) {
+                               int *FileDescriptor) {
   // FIXME: FileSystemOpts shouldn't be passed in here, all paths should be
   // absolute!
   if (FileSystemOpts.WorkingDir.empty())
-    return FileSystemStatCache::get(Path, StatBuf, StatCache.get()) ||
-           S_ISDIR(StatBuf.st_mode) != isForDir;
+    return FileSystemStatCache::get(Path, StatBuf, FileDescriptor,
+                                    StatCache.get());
   
   llvm::sys::Path FilePath(Path);
   FixupRelativePath(FilePath, FileSystemOpts);
 
-  return FileSystemStatCache::get(FilePath.c_str(), StatBuf, StatCache.get()) ||
-         S_ISDIR(StatBuf.st_mode) != isForDir;
+  return FileSystemStatCache::get(FilePath.c_str(), StatBuf, FileDescriptor,
+                                  StatCache.get());
 }
 
 
diff --git a/lib/Basic/FileSystemStatCache.cpp b/lib/Basic/FileSystemStatCache.cpp
index 6457414df9..b2d43e4744 100644
--- a/lib/Basic/FileSystemStatCache.cpp
+++ b/lib/Basic/FileSystemStatCache.cpp
@@ -19,9 +19,35 @@ using namespace clang;
 #define S_ISDIR(s) (_S_IFDIR & s)
 #endif
 
+/// FileSystemStatCache::get - Get the 'stat' information for the specified
+/// path, using the cache to accellerate it if possible.  This returns true if
+/// the path does not exist or false if it exists.
+///
+/// If FileDescriptor is non-null, then this lookup should only return success
+/// for files (not directories).  If it is null this lookup should only return
+/// success for directories (not files).  On a successful file lookup, the
+/// implementation can optionally fill in FileDescriptor with a valid
+/// descriptor and the client guarantees that it will close it.
+bool FileSystemStatCache::get(const char *Path, struct stat &StatBuf,
+                              int *FileDescriptor, FileSystemStatCache *Cache) {
+  LookupResult R;
+  
+  if (Cache)
+    R = Cache->getStat(Path, StatBuf, FileDescriptor);
+  else
+    R = ::stat(Path, &StatBuf) != 0 ? CacheMissing : CacheExists;
+
+  if (R == CacheMissing) return true;
+  
+  bool isForDir = FileDescriptor == 0;
+  return S_ISDIR(StatBuf.st_mode) != isForDir;
+}
+
+
 MemorizeStatCalls::LookupResult
-MemorizeStatCalls::getStat(const char *Path, struct stat &StatBuf) {
-  LookupResult Result = statChained(Path, StatBuf);
+MemorizeStatCalls::getStat(const char *Path, struct stat &StatBuf,
+                           int *FileDescriptor) {
+  LookupResult Result = statChained(Path, StatBuf, FileDescriptor);
   
   // Do not cache failed stats, it is easy to construct common inconsistent
   // situations if we do, and they are not important for PCH performance (which
diff --git a/lib/Frontend/CacheTokens.cpp b/lib/Frontend/CacheTokens.cpp
index 2a7af8a8c3..ae4cdb5063 100644
--- a/lib/Frontend/CacheTokens.cpp
+++ b/lib/Frontend/CacheTokens.cpp
@@ -517,8 +517,9 @@ public:
   StatListener(PTHMap &pm) : PM(pm) {}
   ~StatListener() {}
 
-  LookupResult getStat(const char *Path, struct stat &StatBuf) {
-    LookupResult Result = statChained(Path, StatBuf);
+  LookupResult getStat(const char *Path, struct stat &StatBuf,
+                       int *FileDescriptor) {
+    LookupResult Result = statChained(Path, StatBuf, FileDescriptor);
 
     if (Result == CacheMissing) // Failed 'stat'.
       PM.insert(PTHEntryKeyVariant(Path), PTHEntry());
diff --git a/lib/Lex/PTHLexer.cpp b/lib/Lex/PTHLexer.cpp
index ed068675fe..b6bc7ce488 100644
--- a/lib/Lex/PTHLexer.cpp
+++ b/lib/Lex/PTHLexer.cpp
@@ -678,13 +678,14 @@ public:
 
   ~PTHStatCache() {}
 
-  LookupResult getStat(const char *Path, struct stat &StatBuf) {
+  LookupResult getStat(const char *Path, struct stat &StatBuf,
+                       int *FileDescriptor) {
     // Do the lookup for the file's data in the PTH file.
     CacheTy::iterator I = Cache.find(Path);
 
     // If we don't get a hit in the PTH file just forward to 'stat'.
     if (I == Cache.end())
-      return statChained(Path, StatBuf);
+      return statChained(Path, StatBuf, FileDescriptor);
 
     const PTHStatData &Data = *I;
 
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index dc719a1f93..4994b4944e 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -1074,14 +1074,15 @@ public:
 
   ~ASTStatCache() { delete Cache; }
 
-  LookupResult getStat(const char *Path, struct stat &StatBuf) {
+  LookupResult getStat(const char *Path, struct stat &StatBuf,
+                       int *FileDescriptor) {
     // Do the lookup for the file's data in the AST file.
     CacheTy::iterator I = Cache->find(Path);
 
     // If we don't get a hit in the AST file just forward to 'stat'.
     if (I == Cache->end()) {
       ++NumStatMisses;
-      return statChained(Path, StatBuf);
+      return statChained(Path, StatBuf, FileDescriptor);
     }
 
     ++NumStatHits;