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

Comments

Projects
None yet
3 participants
@josharian
Copy link
Contributor

commented Aug 8, 2016

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

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2016

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2016

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

@gopherbot

This comment has been minimized.

Copy link

commented Aug 22, 2016

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

@gopherbot gopherbot closed this in e80376c Aug 22, 2016

@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.
You can’t perform that action at this time.