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

runtime: closeonexec return value ignored #16641

Closed
josharian opened this issue Aug 8, 2016 · 3 comments
Closed

runtime: closeonexec return value ignored #16641

josharian opened this issue Aug 8, 2016 · 3 comments
Milestone

Comments

@josharian
Copy link
Contributor

Quoting runtime/sys_openbsd_arm.s:

// int32 runtime·closeonexec(int32 fd);
TEXT runtime·closeonexec(SB),NOSPLIT,$0
    MOVW    fd+0(FP), R0        // arg 1 - fd
    MOVW    $2, R1          // arg 2 - cmd (F_SETFD)
    MOVW    $1, R2          // arg 3 - arg (FD_CLOEXEC)
    MOVW    $92, R12        // sys_fcntl
    SWI $0
    RSB.CS  $0, R0
    MOVW    R0, ret+4(FP)
    RET

The comment at the top indicates an int32 return value, and the second to last line confirms that. However, the Go function prototype (runtime/netpoll_{epoll,kqueue}.go) has no return value:

func closeonexec(fd int32)

And openbsd/arm appears to be the only platform in which we attempt to return a value.

Discovered while working on the interminable (but valuable?) #11041.

Input requested. Should we:

  • eliminate the bad ret value write in openbsd/arm
  • accept a return value from closeonexec, update all the other platforms to provide it, and then use it
@josharian josharian added this to the Go1.8 milestone Aug 8, 2016
@ianlancetaylor
Copy link
Contributor

There is no reasonable action the runtime can take if closeonexec fails, other than simply carry on, and there is no actual likelihood of failure, so I think we should consistently not return a value.

@josharian
Copy link
Contributor Author

Great. Thanks, Ian. I'm on it.

@gopherbot
Copy link

CL https://golang.org/cl/27490 mentions this issue.

@golang golang locked and limited conversation to collaborators Aug 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants