Skip to content
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.

Sign up for GitHub

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

Jump to bottom

FoundationEssentials: perform explicit conversion of mode_t #522

Closed
wants to merge 1 commit into from
Closed

FoundationEssentials: perform explicit conversion of mode_t #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions Sources/FoundationEssentials/Data/Data+Reading.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,17 @@ internal func readBytesFromFile(path inPath: PathOrURL, reportProgress: Bool, ma
}

let fileSize = min(Int(clamping: filestat.st_size), maxLength ?? Int.max)
let fileType = filestat.st_mode & S_IFMT
let fileType = mode_t(filestat.st_mode) & mode_t(S_IFMT)
let preferredChunkSize = filestat.st_blksize
#if !NO_FILESYSTEM
let shouldMap = shouldMapFileDescriptor(fd, path: inPath, options: options)
#else
let shouldMap = false
#endif

if fileType != S_IFREG {
if fileType != mode_t(S_IFREG) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For my reference, could you elaborate a bit more on why these casts are all necessary? What type is S_IFREG on Windows? Does it use some other integer type, or does it use int? If so I think I'd expect that declaring a typealias mode_t = Int (or the appropriate integer type) would eliminate the need for these casts because S_IFREG would be considered the same type was fileType here. Or is there a detail I might be missing here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no mode_t on Windows, there is an alias that I have created which is int. However, the type system often sees the values as UInt16 which would then be incompatible with int. The explicit casting avoids the mismatched types on the various sides.

// EACCES is still an odd choice, but at least we have a better error for directories.
let code = (fileType == S_IFDIR) ? EISDIR : EACCES
let code = (fileType == mode_t(S_IFDIR)) ? EISDIR : EACCES
throw CocoaError.errorWithFilePath(inPath, errno: code, reading: true)
}

Expand Down
7 changes: 4 additions & 3 deletions Sources/FoundationEssentials/FileManager/FileManager+Basics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import Darwin
#elseif canImport(Glibc)
import Glibc
#elseif os(Windows)
import CRT
import ucrt
internal import _CShims
#endif

internal struct _FileManagerImpl {
Expand Down Expand Up @@ -64,7 +65,7 @@ internal struct _FileManagerImpl {
var statBuf = stat()
let fd = open(path, 0, 0)
guard fd >= 0 else { return nil }
if fstat(fd, &statBuf) < 0 || statBuf.st_mode & S_IFMT == S_IFDIR {
if fstat(fd, &statBuf) < 0 || mode_t(statBuf.st_mode) & mode_t(S_IFMT) == mode_t(S_IFDIR) {
close(fd)
return nil
}
Expand All @@ -83,7 +84,7 @@ internal struct _FileManagerImpl {
}

/* check for being same type */
if myInfo.st_mode & S_IFMT != otherInfo.st_mode & S_IFMT {
if myInfo.st_mode & mode_t(S_IFMT) != otherInfo.st_mode & mode_t(S_IFMT) {
return false
}

Expand Down
19 changes: 10 additions & 9 deletions Sources/FoundationEssentials/FileManager/FileManager+Files.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Glibc
internal import _CShims
#elseif os(Windows)
import CRT
internal import _CShims
#endif

extension Date {
Expand All @@ -46,13 +47,13 @@ private func _nameFor(gid: gid_t) -> String? {

extension mode_t {
private var _fileType: FileAttributeType {
switch self & S_IFMT {
case S_IFCHR: .typeCharacterSpecial
case S_IFDIR: .typeDirectory
case S_IFBLK: .typeBlockSpecial
case S_IFREG: .typeRegular
case S_IFLNK: .typeSymbolicLink
case S_IFSOCK: .typeSocket
switch self & mode_t(S_IFMT) {
case mode_t(S_IFCHR): .typeCharacterSpecial
case mode_t(S_IFDIR): .typeDirectory
case mode_t(S_IFBLK): .typeBlockSpecial
case mode_t(S_IFREG): .typeRegular
case mode_t(S_IFLNK): .typeSymbolicLink
case mode_t(S_IFSOCK): .typeSocket
default: .typeUnknown
}
}
Expand Down Expand Up @@ -344,7 +345,7 @@ extension _FileManagerImpl {
guard stat(rep, &fileInfo) == 0 else {
return (false, false)
}
let isDir = (fileInfo.st_mode & S_IFMT) == S_IFDIR
let isDir = (mode_t(fileInfo.st_mode) & mode_t(S_IFMT)) == mode_t(S_IFDIR)
return (true, isDir)
}
}
Expand Down Expand Up @@ -644,7 +645,7 @@ extension _FileManagerImpl {
}
}

if mode_t(mode) & S_IWUSR != 0 {
if mode_t(mode) & mode_t(S_IWUSR) != 0 {
try setMode?()
setMode = nil
}
Expand Down
19 changes: 11 additions & 8 deletions Sources/FoundationEssentials/FileManager/FileManager+Utilities.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,28 @@ import Darwin
import Glibc
internal import _CShims
#elseif os(Windows)
import CRT
import ucrt
internal import _CShims
#endif

extension stat {
var isDirectory: Bool {
(self.st_mode & S_IFMT) == S_IFDIR
(mode_t(self.st_mode) & mode_t(S_IFMT)) == mode_t(S_IFDIR)
}

var isRegular: Bool {
(self.st_mode & S_IFMT) == S_IFREG
(mode_t(self.st_mode) & mode_t(S_IFMT)) == mode_t(S_IFREG)
}


#if !os(Windows)
var isSymbolicLink: Bool {
(self.st_mode & S_IFMT) == S_IFLNK
(mode_t(self.st_mode) & mode_t(S_IFMT)) == mode_t(S_IFLNK)
}

#endif

var isSpecial: Bool {
let type = self.st_mode & S_IFMT
return type == S_IFBLK || type == S_IFCHR
let type = mode_t(self.st_mode) & mode_t(S_IFMT)
return type == mode_t(S_IFBLK) || type == mode_t(S_IFCHR)
}
}

Expand Down
3 changes: 2 additions & 1 deletion Sources/FoundationEssentials/FileManager/FileOperations+Enumeration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Glibc
internal import _CShims
#elseif os(Windows)
import CRT
internal import _CShims
#endif

// MARK: Directory Iteration
Expand Down Expand Up @@ -266,7 +267,7 @@ struct _POSIXDirectoryContentsSequence: Sequence {
let statDir = directoryPath + "/" + fileName
if stat(statDir, &statBuf) == 0 {
// #define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
if (statBuf.st_mode & S_IFMT) == S_IFDIR {
if (mode_t(statBuf.st_mode) & mode_t(S_IFMT)) == mode_t(S_IFDIR) {
isDirectory = true
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/FoundationEssentials/FileManager/FileOperations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ enum _FileOperations {
defer { close(dstfd) }

// Set the file permissions using fchmod() instead of when open()ing to avoid umask() issues
let permissions = fileInfo.st_mode & ~S_IFMT
let permissions = mode_t(fileInfo.st_mode) & ~mode_t(S_IFMT)
guard fchmod(dstfd, permissions) == 0 else {
try delegate.throwIfNecessary(errno, srcPtr, dstPtr)
return
Expand Down
2 changes: 1 addition & 1 deletion Sources/FoundationEssentials/String/String+Path.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ extension String {
if lstat(buffer.baseAddress!, &statBuf) < 0 {
return nil
}
if statBuf.st_mode & S_IFMT == S_IFLNK {
if st_mode(statBuf.st_mode) & st_mode(S_IFMT) == st_mode(S_IFLNK) {
/* Examples:
* fspath == /foo/bar0baz/quux/froboz
* linkx == /tic/tac/toe
Expand Down
4 changes: 4 additions & 0 deletions Sources/_CShims/include/platform_shims.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
#include <libkern/OSThermalNotification.h>
#endif

#if defined(_WIN32)
typedef int mode_t;
Copy link
Member

Choose a reason for hiding this comment

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

What does mode_t mean on Windows? Why would we want Foundation to use UNIX/POSIX APIs on Windows?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It means fewer code path divergences. Having a large number of #if os(...) conditionals hampers the ability to maintain the code path. Using the same terminology just is a convenience that is worth it. In general, Windows does have enough of the POSIX specification implemented to qualify as a POSIX compatible environment.

#endif

INTERNAL char * _Nullable * _Nullable _platform_shims_get_environ();

INTERNAL void _platform_shims_lock_environ();
Expand Down