Navigation Menu

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.

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

x/sys/unix: broken FcntlInt #26078

Closed
mikioh opened this issue Jun 27, 2018 · 5 comments
Closed

x/sys/unix: broken FcntlInt #26078

mikioh opened this issue Jun 27, 2018 · 5 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@mikioh
Copy link
Contributor

mikioh commented Jun 27, 2018

FcntlInt returns a non-nil error value regardless of the returned value from the system call.

// FcntlInt performs a fcntl syscall on fd with the provided command and argument.
func FcntlInt(fd uintptr, cmd, arg int) (int, error) {
	valptr, _, err := Syscall(fcntl64Syscall, fd, uintptr(cmd), uintptr(arg))
	return int(valptr), err
}

See https://go-review.googlesource.com/c/sys/+/104736 for further information.
@mikioh mikioh added this to the Unplanned milestone Jun 27, 2018
@gopherbot
Copy link

Change https://golang.org/cl/121175 mentions this issue: unix: do not return non-nil error for 0 errno in FcntlInt

@fazalmajid
Copy link

The test is failing on one of my old OpenIndiana oi151a1 boxes, but not on newer SmartOS ones.

ok      cmd/vendor/golang.org/x/arch/x86/x86asm 0.187s
ok      cmd/vendor/golang.org/x/crypto/ssh/terminal     0.041s
--- FAIL: TestFcntlInt (0.00s)
    syscall_unix_test.go:115: flags 0x0 do not include FD_CLOEXEC
FAIL
FAIL    cmd/vendor/golang.org/x/sys/unix        0.485s
ok      cmd/vet 5.183s
ok      cmd/vet/internal/cfg    0.021s
2018/09/07 21:43:44 Failed: exit status 1
local64 ~/build>uname -a
SunOS local64.apsalar.com 5.11 oi_151a i86pc i386 i86pc

@ianlancetaylor
What's the rationale for checking that the temp file is set FD_CLOEXEC using fcntl F_GETFD? ioutil.TempFile does not set the flag (the flags passed to open() are os.O_RDWR|os.O_CREATE|os.O_EXCL but not O_CLOEXEC)

@ianlancetaylor
Copy link
Contributor

@fazalmajid The os package sets FD_CLOEXEC for every file that it opens. In this case it will happen because the call to syscall.Open in openFileNoLog in os/file_unix.go adds syscall.O_CLOEXEC to the flags passed to syscall.Open. Could your OS be so old that it doesn't support passing O_CLOEXECtoopen`? And doesn't even return an error in that case?

@ianlancetaylor
Copy link
Contributor

@fazalmajid In any case this should probably be discussed in a new issue as it is not related to the topic of this issue. Thanks.

@fazalmajid
Copy link

Yes, O_CLOEXEC was only added to Illumos in 2013, and the system in question dates to 2010 or 2011. It's not worth creating a new issue for OS versions that ancient.

@golang golang locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@mikioh @fazalmajid @ianlancetaylor @gopherbot