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

Fix close error message #433

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Fix close error message #433

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9171044
Fix close error message
neonichu Nov 2, 2023
8d4e281
Update Sources/TSCBasic/misc.swift
neonichu Nov 2, 2023
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
10 changes: 8 additions & 2 deletions Sources/TSCBasic/misc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,14 @@ extension SystemError: CustomStringConvertible {
switch self {
case .chdir(let errno, let path):
return "chdir error: \(strerror(errno)): \(path)"
case .close(let errno):
return "close error: \(strerror(errno))"
case .close(let err):
let errorMessage: String
if err == -1 { // if the return code is -1, we need to consult the global `errno`
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, errno is only guaranteed to overridden when the syscall fails, i.e. returns -1.

errorMessage = strerror(errno)
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to copy this string immediately or else it's UB and you might get garbage or worse:

man strerror:

BUGS
For unknown error numbers, the strerror() function will return its result
in a static buffer which may be overwritten by subsequent calls.

So you want to save a String(cString: strerror(errno)) + " (\(errno))" or something. I'd say let's not lose the actual errno number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we are basically doing that, the strerror here is not the libc function, but a local one https://github.com/apple/swift-tools-support-core/blob/main/Sources/TSCBasic/misc.swift#L320-L345

} else {
errorMessage = strerror(err)
}
return "close error: \(errorMessage)"
case .exec(let errno, let path, let args):
let joinedArgs = args.joined(separator: " ")
return "exec error: \(strerror(errno)): \(path) \(joinedArgs)"
Expand Down