Remove unnecessary casts within FileManager #1594
Conversation
|
As the title suggests there were a couple unnecessary casts typically between int types in FileManager.swift. These are fixed in this pr. |
Foundation/FileManager.swift
Outdated
| @@ -503,16 +503,16 @@ open class FileManager : NSObject { | |||
| let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(fileInfo.st_blksize)) | |||
| defer { buffer.deallocate() } | |||
|
|
|||
| var bytesRemaining = Int64(fileInfo.st_size) | |||
| var bytesRemaining = Int(fileInfo.st_size) | |||
| while bytesRemaining > 0 { | |||
There was a problem hiding this comment.
st_size might be 64bits even on 32bit platforms, so this avoids any problems by always converting upto 64bits. The rest of the casts in this section are so that all arithmetic is conducted in 64bits
There was a problem hiding this comment.
Oh I didn't consider this. So is the cast down from Int64 to Int in the following line safe on 32bit platforms because fileInfo.st_blksize is Int32 on all platforms?
let bytesRead = try _readFrom(fd: srcfd, toBuffer: buffer, length: Int(bytesToRead), filename: srcPath)There was a problem hiding this comment.
Also would is this just implicit knowledge someone should know or would it be prudent to include in a comment?
There was a problem hiding this comment.
Yes, its unlikely st_blksize will be 64bit on any platform and even if it was, its value would be unlikely to exceed Int32.max. If it ever is, it will trap and abort the program and can be dealt with then.
bytesRemaining is computed in 64bits as file size can be large, hence bytesToRead also needs to be computed in 64bits. However the blocks that are read/written will be a lot smaller and read() takes an int hence the cast.
Foundation/FileManager.swift
Outdated
| let len = Int(PATH_MAX) + 1 | ||
| var buf = [Int8](repeating: 0, count: len) | ||
| getcwd(&buf, len) | ||
| return string(withFileSystemRepresentation: buf, length: len) | ||
| } |
There was a problem hiding this comment.
string(withFileSystemRepresentation:length:) takes the length of the NUL-terminated string, not the whole buffer so strlen(buf) is correct here - although destinationOfSymbolicLink(atPath:) seems to be written the wrong way round so needs fixing.
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
No description provided.