FileManager fixes

- Use a common subfunction to read directory contents in
  contentsOfDirectory(atPath:) and subpathsOfDirectory(atPath:).

- Use readdir_r() instead of readdir().

- Call fileSystemRepresentation(withPath:) on the requested directory
  to match behaviour on Darwin.

- Replace the test case of a bad directory using "" with "/..." which
  now fails the precondition (to match Darwin).

Author: spevans

CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h

@@ -27,6 +27,7 @@
 #include <CoreFoundation/ForFoundationOnly.h>
 #include <fts.h>
 #include <pthread.h>
+#include <dirent.h>
 
 #if __has_include(<execinfo.h>)
 #include <execinfo.h>
@@ -399,6 +400,13 @@ static inline _Bool _withStackOrHeapBuffer(size_t amount, void (__attribute__((n
     return true;
 }
 
+static inline int _direntNameLength(struct dirent *entry) {
+#ifdef _D_EXACT_NAMLEN  // defined on Linux
+    return _D_EXACT_NAMLEN(entry);
+#else
+    return entry->d_namlen;
+#endif
+}
 
 _CF_EXPORT_SCOPE_END
 

Foundation/FileManager.swift

@@ -171,7 +171,35 @@ open class FileManager : NSObject {
             }
         }
     }
-    
+
+    private func _contentsOfDir(atPath path: String, _ closure: (String, Int32) throws -> () ) throws {
+        let fsRep = fileSystemRepresentation(withPath: path)
+        defer { fsRep.deallocate() }
+
+        guard let dir = opendir(fsRep) else {
+            throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadNoSuchFile.rawValue, userInfo: [NSFilePathErrorKey: path])
+        }
+        defer { closedir(dir) }
+
+        var entry = dirent()
+        var result: UnsafeMutablePointer<dirent>? = nil
+
+        while readdir_r(dir, &entry, &result) == 0 {
+            guard result != nil else {
+                return
+            }
+            let length = Int(_direntNameLength(&entry))
+            let entryName = withUnsafePointer(to: &entry.d_name) { (ptr) -> String in
+                let namePtr = UnsafeRawPointer(ptr).assumingMemoryBound(to: CChar.self)
+                return string(withFileSystemRepresentation: namePtr, length: length)
+            }
+            if entryName != "." && entryName != ".." {
+                let entryType = Int32(entry.d_type)
+                try closure(entryName, entryType)
+            }
+        }
+    }
+
     /**
      Performs a shallow search of the specified directory and returns the paths of any contained items.
 
@@ -186,28 +214,11 @@ open class FileManager : NSObject {
      - Returns: An array of String each of which identifies a file, directory, or symbolic link contained in `path`. The order of the files returned is undefined.
      */
     open func contentsOfDirectory(atPath path: String) throws -> [String] {
-        var contents : [String] = [String]()
-        
-        let dir = opendir(path)
-        
-        if dir == nil {
-            throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadNoSuchFile.rawValue, userInfo: [NSFilePathErrorKey: path])
-        }
-        
-        defer {
-            closedir(dir!)
-        }
+        var contents: [String] = []
 
-        while let entry = readdir(dir!) {
-            let entryName = withUnsafePointer(to: &entry.pointee.d_name) {
-                String(cString: UnsafeRawPointer($0).assumingMemoryBound(to: CChar.self))
-            }
-            // TODO: `entryName` should be limited in length to `entry.memory.d_namlen`.
-            if entryName != "." && entryName != ".." {
-                contents.append(entryName)
-            }
-        }
-        
+        try _contentsOfDir(atPath: path, { (entryName, entryType) throws in
+            contents.append(entryName)
+        })
         return contents
     }
 
@@ -225,48 +236,16 @@ open class FileManager : NSObject {
     - Returns: An array of NSString objects, each of which contains the path of an item in the directory specified by path. If path is a symbolic link, this method traverses the link. This method returns nil if it cannot retrieve the device of the linked-to file.
     */
     open func subpathsOfDirectory(atPath path: String) throws -> [String] {
-        var contents : [String] = [String]()
-        
-        let dir = opendir(path)
-        
-        if dir == nil {
-            throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadNoSuchFile.rawValue, userInfo: [NSFilePathErrorKey: path])
-        }
-        
-        defer {
-            closedir(dir!)
-        }
-        
-        var entry = readdir(dir!)
-        
-        while entry != nil {
-            let entryName = withUnsafePointer(to: &entry!.pointee.d_name) {
-                String(cString: UnsafeRawPointer($0).assumingMemoryBound(to: CChar.self))
-            }
-            // TODO: `entryName` should be limited in length to `entry.memory.d_namlen`.
-            if entryName != "." && entryName != ".." {
-                contents.append(entryName)
-                    
-                let entryType = withUnsafePointer(to: &entry!.pointee.d_type) { (ptr) -> Int32 in
-                    return Int32(ptr.pointee)
-                }
-                #if os(OSX) || os(iOS)
-                    let tempEntryType = entryType
-                #elseif os(Linux) || os(Android) || CYGWIN
-                    let tempEntryType = Int32(entryType)
-                #endif
+        var contents: [String] = []
 
-                if tempEntryType == Int32(DT_DIR) {
-                    let subPath: String = path + "/" + entryName
-
-                    let entries =  try subpathsOfDirectory(atPath: subPath)
-                    contents.append(contentsOf: entries.map({file in "\(entryName)/\(file)"}))
-                }
+        try _contentsOfDir(atPath: path, { (entryName, entryType) throws in
+            contents.append(entryName)
+            if entryType == Int32(DT_DIR) {
+                let subPath: String = path + "/" + entryName
+                let entries = try subpathsOfDirectory(atPath: subPath)
+                contents.append(contentsOf: entries.map({file in "\(entryName)/\(file)"}))
             }
-            
-            entry = readdir(dir!)
-        }
-        
+        })
         return contents
     }
 

TestFoundation/TestFileManager.swift

@@ -329,7 +329,7 @@ class TestFileManager : XCTestCase {
         } catch _ {
             XCTFail()
         }
-        
+
         if let e = FileManager.default.enumerator(at: URL(fileURLWithPath: path), includingPropertiesForKeys: nil, options: [], errorHandler: nil) {
             var foundItems = [String:Int]()
             while let item = e.nextObject() as? URL {
@@ -442,8 +442,8 @@ class TestFileManager : XCTestCase {
         }
 
         do {
-            let _ = try fm.contentsOfDirectory(atPath: "")
-            
+            // Check a bad path fails
+            let _ = try fm.contentsOfDirectory(atPath: "/...")
             XCTFail()
         }
         catch _ {
@@ -492,8 +492,8 @@ class TestFileManager : XCTestCase {
         }
 
         do {
-            let _ = try fm.subpathsOfDirectory(atPath: "")
-            
+            // Check a bad path fails
+            let _ = try fm.subpathsOfDirectory(atPath: "/...")
             XCTFail()
         }
         catch _ {