Skip to content

FileManager fixes #1481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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

Expand Down
103 changes: 41 additions & 62 deletions Foundation/FileManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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
}

Expand All @@ -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
}

Expand Down
10 changes: 5 additions & 5 deletions TestFoundation/TestFileManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -442,8 +442,8 @@ class TestFileManager : XCTestCase {
}

do {
let _ = try fm.contentsOfDirectory(atPath: "")

// Check a bad path fails
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change here, for the record?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its for compatibility with Darwin's native Foundation. This call will throw an exception under native Foundation as the filename is passed through fileSystemRepresentation(withPath:) which treats nil and "" as invalid paths. This PR modifies contentsOfDirectory(atPath:) and subpathsOfDirectory(atPath:) to now also pass the path through this function so "" is now going to fail the precondition(path != "", "Empty path argument") check.

FYI, if you open the Xcode project DarwinCompatibilityTests in the swift-corelibs-foundation directory you can run all of the tests as unit tests against native Foundation to see what differences currently exist.

let _ = try fm.contentsOfDirectory(atPath: "/...")
XCTFail()
}
catch _ {
Expand Down Expand Up @@ -492,8 +492,8 @@ class TestFileManager : XCTestCase {
}

do {
let _ = try fm.subpathsOfDirectory(atPath: "")

// Check a bad path fails
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above.

let _ = try fm.subpathsOfDirectory(atPath: "/...")
XCTFail()
}
catch _ {
Expand Down