From: Rafael Espindola <rafael.espindola@gmail.com>
Date: Thu, 27 Jun 2013 18:26:26 +0000 (+0000)
Subject: Small improvements to createOutputFile.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=375a4f519eb85d37e702c90498ef9816aeda4c3e;p=clang

Small improvements to createOutputFile.

* Use a single stat to find out if the file exists and if it is a regular file.
* Use early returns when possible.
* Add comments explaining why we have each check.

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

diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h
index 3ad05a6510..5673c5908d 100644
--- a/include/clang/Frontend/CompilerInstance.h
+++ b/include/clang/Frontend/CompilerInstance.h
@@ -591,10 +591,10 @@ public:
   /// \return - Null on error.
   llvm::raw_fd_ostream *
   createOutputFile(StringRef OutputPath,
-                   bool Binary = true, bool RemoveFileOnSignal = true,
-                   StringRef BaseInput = "",
-                   StringRef Extension = "",
-                   bool UseTemporary = false,
+                   bool Binary, bool RemoveFileOnSignal,
+                   StringRef BaseInput,
+                   StringRef Extension,
+                   bool UseTemporary,
                    bool CreateMissingDirectories = false);
 
   /// Create a new output file, optionally deriving the output path name.
@@ -624,13 +624,13 @@ public:
   /// will be stored here on success.
   static llvm::raw_fd_ostream *
   createOutputFile(StringRef OutputPath, std::string &Error,
-                   bool Binary = true, bool RemoveFileOnSignal = true,
-                   StringRef BaseInput = "",
-                   StringRef Extension = "",
-                   bool UseTemporary = false,
-                   bool CreateMissingDirectories = false,
-                   std::string *ResultPathName = 0,
-                   std::string *TempPathName = 0);
+                   bool Binary, bool RemoveFileOnSignal,
+                   StringRef BaseInput,
+                   StringRef Extension,
+                   bool UseTemporary,
+                   bool CreateMissingDirectories,
+                   std::string *ResultPathName,
+                   std::string *TempPathName);
 
   /// }
   /// @name Initialization Utility Methods
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index 4bd398e9cc..ec1f959060 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -519,34 +519,50 @@ CompilerInstance::createOutputFile(StringRef OutputPath,
   OwningPtr<llvm::raw_fd_ostream> OS;
   std::string OSFile;
 
-  if (UseTemporary && OutFile != "-") {
-    // Only create the temporary if the parent directory exists (or create
-    // missing directories is true) and we can actually write to OutPath,
-    // otherwise we want to fail early.
+  if (UseTemporary) {
+    if (OutFile == "-")
+      UseTemporary = false;
+    else {
+      llvm::sys::fs::file_status Status;
+      llvm::sys::fs::status(OutputPath, Status);
+      if (llvm::sys::fs::exists(Status)) {
+        // Fail early if we can't write to the final destination.
+        if (!llvm::sys::fs::can_write(OutputPath))
+          return 0;
+
+        // Don't use a temporary if the output is a special file. This handles
+        // things like '-o /dev/null'
+        if (!llvm::sys::fs::is_regular_file(Status))
+          UseTemporary = false;
+      }
+    }
+  }
+
+  if (UseTemporary) {
     SmallString<256> AbsPath(OutputPath);
     llvm::sys::fs::make_absolute(AbsPath);
-    StringRef OutPath = AbsPath;
-    bool ParentExists = false;
-    if (llvm::sys::fs::exists(llvm::sys::path::parent_path(AbsPath.str()),
-                              ParentExists))
-      ParentExists = false;
-    bool Exists;
-    if ((CreateMissingDirectories || ParentExists) &&
-        ((llvm::sys::fs::exists(AbsPath.str(), Exists) || !Exists) ||
-         (llvm::sys::fs::is_regular_file(OutPath) &&
-          llvm::sys::fs::can_write(AbsPath.c_str())))) {
-      // Create a temporary file.
-      SmallString<128> TempPath;
-      TempPath = OutFile;
-      TempPath += "-%%%%%%%%";
-      int fd;
-      if (llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
-                                     /*makeAbsolute=*/false, 0664)
-          == llvm::errc::success) {
-        OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
-        OSFile = TempFile = TempPath.str();
-      }
+
+    // If the parent directory doesn't exist and we can't create it, fail.
+    bool ParentExists =
+        llvm::sys::fs::exists(llvm::sys::path::parent_path(AbsPath.str()));
+    if (!CreateMissingDirectories && !ParentExists) {
+      Error = "Parent directory doesn't exist";
+      return 0;
+    }
+
+    // Create a temporary file.
+    SmallString<128> TempPath;
+    TempPath = OutFile;
+    TempPath += "-%%%%%%%%";
+    int fd;
+    if (!llvm::sys::fs::unique_file(TempPath.str(), fd, TempPath,
+                                   /*makeAbsolute=*/false, 0664)) {
+      OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
+      OSFile = TempFile = TempPath.str();
     }
+    // If we failed to create the temporary, fallback to writing to the file
+    // directly. This handles the corner case where we cannot write to the
+    // directory, but can write to the file.
   }
 
   if (!OS) {
diff --git a/test/Frontend/output-failures.c b/test/Frontend/output-failures.c
index e2af7c7ddc..2b5aba6ab9 100644
--- a/test/Frontend/output-failures.c
+++ b/test/Frontend/output-failures.c
@@ -1,4 +1,4 @@
 // RUN: not %clang_cc1 -emit-llvm -o %S/doesnotexist/somename %s 2> %t
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -input-file=%t %s
 
-// OUTPUTFAIL: Error opening output file '{{.*}}doesnotexist{{.*}}'
+// OUTPUTFAIL:  unable to open output file '{{.*}}doesnotexist{{.*}}': 'Parent directory doesn't exist'