[cfe-commits] r127748 - in /cfe/trunk: include/clang/Basic/FileManager.h lib/Basic/FileManager.cpp lib/Lex/HeaderSearch.cpp

[Argyrios Kyrtzidis]

Author: akirtzidis
Date: Wed Mar 16 14:17:25 2011
New Revision: 127748

URL: http://llvm.org/viewvc/llvm-project?rev=127748&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Basic/FileManager.h
    cfe/trunk/lib/Basic/FileManager.cpp
    cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=127748&r1=127747&r2=127748&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Wed Mar 16 14:17:25 2011
@@ -176,10 +176,11 @@
   ///
   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

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=127748&r1=127747&r2=127748&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Wed Mar 16 14:17:25 2011
@@ -313,7 +313,7 @@
 /// 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 @@
     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);

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=127748&r1=127747&r2=127748&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Mar 16 14:17:25 2011
@@ -126,7 +126,7 @@
     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 @@
 
   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 @@
                        Private+strlen(Private));
   if (RawPath != NULL)
     *RawPath = FrameworkName;
-  return FileMgr.getFile(FrameworkName.str());
+  return FileMgr.getFile(FrameworkName.str(), /*openFile=*/true);
 }
 
 
@@ -235,7 +236,7 @@
     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 @@
     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 @@
   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)