From: Argyrios Kyrtzidis <akyrtzi@gmail.com>
Date: Wed, 16 Mar 2011 19:17:25 +0000 (+0000)
Subject: Having FileManager::getFile always open the file, brought much consternation and... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3cd0128ce49abe658d1858c541e836e57959e04a;p=clang

Having FileManager::getFile always open the file, brought much consternation and leaking of file descriptors.

Add 'openFile' bool to FileManager::getFile to specify whether we want to have the file opened or not, have it
false by default, and enable it only in HeaderSearch.cpp where the open+fstat optimization matters.

Fixes rdar://9139899.

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

diff --git a/include/clang/Basic/FileManager.h b/include/clang/Basic/FileManager.h
index 57f7d71660..40f6d86563 100644
--- a/include/clang/Basic/FileManager.h
+++ b/include/clang/Basic/FileManager.h
@@ -176,10 +176,11 @@ public:
   ///
   const DirectoryEntry *getDirectory(llvm::StringRef DirName);
 
-  /// getFile - Lookup, cache, and verify the specified file (real or
+  /// \brief Lookup, cache, and verify the specified file (real or
   /// virtual).  This returns NULL if the file doesn't exist.
   ///
-  const FileEntry *getFile(llvm::StringRef Filename);
+  /// \param openFile if true and the file exists, it will be opened.
+  const FileEntry *getFile(llvm::StringRef Filename, bool openFile = false);
 
   /// \brief Retrieve a file entry for a "virtual" file that acts as
   /// if there were a file with the given name on disk. The file
diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
index 9ad6e51a83..1a8b01938e 100644
--- a/lib/Basic/FileManager.cpp
+++ b/lib/Basic/FileManager.cpp
@@ -313,7 +313,7 @@ const DirectoryEntry *FileManager::getDirectory(llvm::StringRef DirName) {
 /// getFile - Lookup, cache, and verify the specified file (real or
 /// virtual).  This returns NULL if the file doesn't exist.
 ///
-const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
+const FileEntry *FileManager::getFile(llvm::StringRef Filename, bool openFile) {
   ++NumFileLookups;
 
   // See if there is already an entry in the map.
@@ -354,6 +354,11 @@ const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
     return 0;
   }
 
+  if (FileDescriptor != -1 && !openFile) {
+    close(FileDescriptor);
+    FileDescriptor = -1;
+  }
+
   // It exists.  See if we have already opened a file with the same inode.
   // This occurs when one dir is symlinked to another, for example.
   FileEntry &UFE = UniqueRealFiles.getFile(InterndFileName, StatBuf);
diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp
index bb4ff60480..db91ba4e78 100644
--- a/lib/Lex/HeaderSearch.cpp
+++ b/lib/Lex/HeaderSearch.cpp
@@ -126,7 +126,7 @@ const FileEntry *DirectoryLookup::LookupFile(
     TmpDir.append(Filename.begin(), Filename.end());
     if (RawPath != NULL)
       *RawPath = TmpDir;
-    return HS.getFileMgr().getFile(TmpDir.str());
+    return HS.getFileMgr().getFile(TmpDir.str(), /*openFile=*/true);
   }
 
   if (isFramework())
@@ -192,7 +192,8 @@ const FileEntry *DirectoryLookup::DoFrameworkLookup(
 
   FrameworkName += "Headers/";
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
-  if (const FileEntry *FE = FileMgr.getFile(FrameworkName.str())) {
+  if (const FileEntry *FE = FileMgr.getFile(FrameworkName.str(),
+                                            /*openFile=*/true)) {
     if (RawPath != NULL)
       *RawPath = FrameworkName;
     return FE;
@@ -204,7 +205,7 @@ const FileEntry *DirectoryLookup::DoFrameworkLookup(
                        Private+strlen(Private));
   if (RawPath != NULL)
     *RawPath = FrameworkName;
-  return FileMgr.getFile(FrameworkName.str());
+  return FileMgr.getFile(FrameworkName.str(), /*openFile=*/true);
 }
 
 
@@ -235,7 +236,7 @@ const FileEntry *HeaderSearch::LookupFile(
     if (RawPath != NULL)
       llvm::Twine(Filename).toVector(*RawPath);
     // Otherwise, just return the file.
-    return FileMgr.getFile(Filename);
+    return FileMgr.getFile(Filename, /*openFile=*/true);
   }
 
   // Step #0, unless disabled, check to see if the file is in the #includer's
@@ -250,7 +251,7 @@ const FileEntry *HeaderSearch::LookupFile(
     TmpDir += CurFileEnt->getDir()->getName();
     TmpDir.push_back('/');
     TmpDir.append(Filename.begin(), Filename.end());
-    if (const FileEntry *FE = FileMgr.getFile(TmpDir.str())) {
+    if (const FileEntry *FE = FileMgr.getFile(TmpDir.str(),/*openFile=*/true)) {
       // Leave CurDir unset.
       // This file is a system header or C++ unfriendly if the old file is.
       //
@@ -376,13 +377,13 @@ LookupSubframeworkHeader(llvm::StringRef Filename,
   llvm::SmallString<1024> HeadersFilename(FrameworkName);
   HeadersFilename += "Headers/";
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  if (!(FE = FileMgr.getFile(HeadersFilename.str()))) {
+  if (!(FE = FileMgr.getFile(HeadersFilename.str(), /*openFile=*/true))) {
 
     // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
     HeadersFilename = FrameworkName;
     HeadersFilename += "PrivateHeaders/";
     HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-    if (!(FE = FileMgr.getFile(HeadersFilename.str())))
+    if (!(FE = FileMgr.getFile(HeadersFilename.str(), /*openFile=*/true)))
       return 0;
   }
   if (RawPath != NULL)