[llvm-branch-commits] [lldb] r199920 - Make lldb_private::File safe to use when mixing "FILE *" and "fd" usage where if we call fdopen, we don't want to close the m_descriptor.

[Greg Clayton]

Author: gclayton
Date: Thu Jan 23 12:53:01 2014
New Revision: 199920

URL: http://llvm.org/viewvc/llvm-project?rev=199920&view=rev
Log:
Make lldb_private::File safe to use when mixing "FILE *" and "fd" usage where if we call fdopen, we don't want to close the m_descriptor.


Modified:
    lldb/branches/iohandler/include/lldb/Host/File.h
    lldb/branches/iohandler/source/Host/common/File.cpp

Modified: lldb/branches/iohandler/include/lldb/Host/File.h
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/iohandler/include/lldb/Host/File.h?rev=199920&r1=199919&r2=199920&view=diff
==============================================================================
--- lldb/branches/iohandler/include/lldb/Host/File.h (original)
+++ lldb/branches/iohandler/include/lldb/Host/File.h Thu Jan 23 12:53:01 2014
@@ -51,7 +51,8 @@ public:
         m_descriptor (kInvalidDescriptor),
         m_stream (kInvalidStream),
         m_options (0),
-        m_owned (false)
+        m_own_stream (false),
+        m_own_descriptor (false)
     {
     }
     
@@ -59,7 +60,8 @@ public:
         m_descriptor (kInvalidDescriptor),
         m_stream (fh),
         m_options (0),
-        m_owned (transfer_ownership)
+        m_own_stream (transfer_ownership),
+        m_own_descriptor (false)
     {
     }
 
@@ -111,13 +113,15 @@ public:
           uint32_t options,
           uint32_t permissions = lldb::eFilePermissionsFileDefault);
     
-    File (int fd, bool tranfer_ownership) : 
+    File (int fd, bool transfer_ownership) :
         m_descriptor (fd),
         m_stream (kInvalidStream),
         m_options (0),
-        m_owned (tranfer_ownership)
+        m_own_stream (false),
+        m_own_descriptor (transfer_ownership)
     {
     }
+
     //------------------------------------------------------------------
     /// Destructor.
     ///
@@ -503,7 +507,8 @@ protected:
     int m_descriptor;
     FILE *m_stream;
     uint32_t m_options;
-    bool m_owned;
+    bool m_own_stream;
+    bool m_own_descriptor;
 };
 
 } // namespace lldb_private

Modified: lldb/branches/iohandler/source/Host/common/File.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/branches/iohandler/source/Host/common/File.cpp?rev=199920&r1=199919&r2=199920&view=diff
==============================================================================
--- lldb/branches/iohandler/source/Host/common/File.cpp (original)
+++ lldb/branches/iohandler/source/Host/common/File.cpp Thu Jan 23 12:53:01 2014
@@ -77,7 +77,8 @@ File::File(const char *path, uint32_t op
     m_descriptor (kInvalidDescriptor),
     m_stream (kInvalidStream),
     m_options (),
-    m_owned (false)
+    m_own_stream (false),
+    m_own_descriptor (false)
 {
     Open (path, options, permissions);
 }
@@ -88,7 +89,8 @@ File::File (const FileSpec& filespec,
     m_descriptor (kInvalidDescriptor),
     m_stream (kInvalidStream),
     m_options (0),
-    m_owned (false)
+    m_own_stream (false),
+    m_own_descriptor (false)
 {
     if (filespec)
     {
@@ -100,7 +102,8 @@ File::File (const File &rhs) :
     m_descriptor (kInvalidDescriptor),
     m_stream (kInvalidStream),
     m_options (0),
-    m_owned (false)
+    m_own_stream (false),
+    m_own_descriptor (false)
 {
     Duplicate (rhs);
 }
@@ -141,7 +144,7 @@ File::SetDescriptor (int fd, bool transf
     if (IsValid())
         Close();
     m_descriptor = fd;
-    m_owned = transfer_ownership;
+    m_own_descriptor = transfer_ownership;
 }
 
 
@@ -155,10 +158,31 @@ File::GetStream ()
             const char *mode = GetStreamOpenModeFromOptions (m_options);
             if (mode)
             {
+                if (!m_own_descriptor)
+                {
+                    // We must duplicate the file descriptor if we don't own it because
+                    // when you call fdopen, the stream will own the fd
+#ifdef _WIN32
+                    m_descriptor = ::_dup(GetDescriptor());
+#else
+                    m_descriptor = ::fcntl(GetDescriptor(), F_DUPFD);
+#endif
+                    m_own_descriptor = true;
+                }
+
                 do
                 {
                     m_stream = ::fdopen (m_descriptor, mode);
                 } while (m_stream == NULL && errno == EINTR);
+
+                // If we got a stream, then we own the stream and should no
+                // longer own the descriptor because fclose() will close it for us
+
+                if (m_stream)
+                {
+                    m_own_stream = true;
+                    m_own_descriptor = false;
+                }
             }
         }
     }
@@ -172,7 +196,7 @@ File::SetStream (FILE *fh, bool transfer
     if (IsValid())
         Close();
     m_stream = fh;
-    m_owned = transfer_ownership;
+    m_own_stream = transfer_ownership;
 }
 
 Error
@@ -194,7 +218,7 @@ File::Duplicate (const File &rhs)
         else
         {
             m_options = rhs.m_options;
-            m_owned = true;
+            m_own_descriptor = true;
         }
     }
     else
@@ -273,7 +297,7 @@ File::Open (const char *path, uint32_t o
         error.SetErrorToErrno();
     else
     {
-        m_owned = true;
+        m_own_descriptor = true;
         m_options = options;
     }
     
@@ -331,27 +355,22 @@ Error
 File::Close ()
 {
     Error error;
-    if (IsValid ())
+    if (StreamIsValid() && m_own_stream)
     {
-        if (m_owned)
-        {
-            if (StreamIsValid())
-            {
-                if (::fclose (m_stream) == EOF)
-                    error.SetErrorToErrno();
-            }
-            
-            if (DescriptorIsValid())
-            {
-                if (::close (m_descriptor) != 0)
-                    error.SetErrorToErrno();
-            }
-        }
-        m_descriptor = kInvalidDescriptor;
-        m_stream = kInvalidStream;
-        m_options = 0;
-        m_owned = false;
+        if (::fclose (m_stream) == EOF)
+            error.SetErrorToErrno();
+    }
+    
+    if (DescriptorIsValid() && m_own_descriptor)
+    {
+        if (::close (m_descriptor) != 0)
+            error.SetErrorToErrno();
     }
+    m_descriptor = kInvalidDescriptor;
+    m_stream = kInvalidStream;
+    m_options = 0;
+    m_own_stream = false;
+    m_own_descriptor = false;
     return error;
 }