x/sys/unix: Reboot takes int but LINUX_REBOOT_CMD_HALT overflows int

[tv42]

package main

import "golang.org/x/sys/unix"

func main() {
    unix.Reboot(unix.LINUX_REBOOT_CMD_HALT)
}

$ GOARCH=amd64 go build halt.go 
$ GOARCH=386 go build halt.go 
# command-line-arguments
./halt.go:6: constant 3454992675 overflows int
$ 

[minux]

you need to cast it to an int.

When generating the constants, there is no way to know whether it's
intended as unsigned constant or signed, esp. those magic numbers.

You might argue that we can change the constants to int, but please
note that you can also use it in this way:
unix.Syscall(unix.SYS_REBOOT, unix.LINUX_REBOOT_CMD_HALT, 0, 0)

And in this case, if the constant were signed, compiler would warn about
the negative constant overflows uintptr.

A more realistic change would be to change unix.Reboot to take a
uintptr. But Reboot is such a uncommon API, I don't think it's worth a
API change just to remove a cast.

[tv42]

Actually, that can't even be converted to int conveniently; the compiler is too smart, and sees the large constant. Jumping through hoops is required:

    h := uint(unix.LINUX_REBOOT_CMD_HALT)
    unix.Reboot(int(h))

[minux]

If we make the const signed, then using Syscall will require the same
conversions. And as I've said, when generating the constants, we don't
know they are signed or unsigned, and I don't want to add manual rules
to the process.

[griesemer]

const c = unix.LINUX_REBOOT_CMD_HALT
unix.Reboot(-(c>>31)<<31 | c&(1<<31-1))

:-)

[minux]

Cool!
Robert, will you write a "fun with Go constants" article/blog some day?

[tv42]

Why does Reboot have to take an int? I understand it's currently just machine generated, but sys/unix has human-written code already. And if you're saying this kind of Go-friendliness doesn't belong in sys/unix that's just really raw syscalls, well where does it belong?

[minux]

The consts are machine generated, but Reboot function is manually defined
in https://golang.org/cl/4256060.

The question is really whether we should make a breaking change for
convenience
reasons. (In retrospect, we probably don't need to add that CL, as that
doesn't
expose all functionality of the underlying reboot syscall, and it's just as
easy
to use Syscall6 directly instead for such a rarely used syscall.)