syscall: linux F_SETLK constant is wrong

[bradfitz]

32-bit Linux kernels have two system calls for just about anything that has a file size
or seek offset that can be over 4GB.

For instance, here is fcntl vs fcntl64 on linux/386 and linux/arm.  Note the correctly
differing syscall numbers:

zsysnum_linux_386.go:   SYS_FCNTL                  = 55
zsysnum_linux_386.go:   SYS_FCNTL64                = 221

zsysnum_linux_arm.go:   SYS_FCNTL                  = 55
zsysnum_linux_arm.go:   SYS_FCNTL64                = 221

But with fcntl, you need to use F_SETLK.  For fnctl64 you have to use F_SETLK64.  Note
that they are all the same:

zerrors_linux_386.go:   F_SETLK                          = 0xd
zerrors_linux_386.go:   F_SETLK64                        = 0xd
zerrors_linux_386.go:   F_SETLKW                         = 0xe
zerrors_linux_386.go:   F_SETLKW64                       = 0xe

zerrors_linux_arm.go:   F_SETLK                          = 0xd
zerrors_linux_arm.go:   F_SETLK64                        = 0xd
zerrors_linux_arm.go:   F_SETLKW                         = 0xe
zerrors_linux_arm.go:   F_SETLKW64                       = 0xe

Unfortunately, F_SETLK is actually 6, not 0xd.

I don't imagine we can fix this at this point, unless this is a regression.

[bradfitz]

Comment 1:

Nope, always wrong, even back to r60 and before.

[davecheney]

Comment 2:

Can we really not change the value ? This falls into the category of a bug, yes ?

[bradfitz]

Comment 3:

Maybe?

[bradfitz]

Comment 4:

It would break anybody on 32-bit platforms using F_SETLK incorrectly where they had
meant F_SETLK64.  For instance, this bad program would start breaking:
        k := struct {
                Type   uint32
                Whence uint32
        Start  uint64
                Len    uint64
                Pid    uint32
    }{
                Type:   syscall.F_WRLCK,
                Whence: uint32(os.SEEK_SET),
                Start:  0,
                Len:    0, // whole file
                Pid:    uint32(os.Getpid()),
    }
    _, _, errno := syscall.Syscall(syscall.SYS_FCNTL64, f.Fd(), 
                 uintptr(syscall.F_SETLK),    // <--- whoops. User meant F_SETLK64.
                 uintptr(unsafe.Pointer(&k)))

[bradfitz]

Comment 5:

... but I suppose we could just document in the release notes that this bug was fixed.  
We could even audit all open source repos for it ahead of time and fix them before Go
1.3.

[ianlancetaylor]

Comment 6:

On GNU/Linux F_GETLK vs. F_GETLK64 depends not on the system call number, but on the
version of the flock struct that is used.  That struct has Start and Len fields that (on
a 32 bit system) are either 32 bit (for F_GETLK) or 64 bit (for F_GETLK64).  So I think
the right thing to do is to implement Flock in the syscall package, and define it to
match the version of F_GETLK.  That should happen automatically if we add Flock to
pkg/syscall/types_linux.go.

[bradfitz]

Comment 7:

https://golang.org/cl/53350043

Labels changed: added repo-main, release-go1.3.

Owner changed to @bradfitz.

Status changed to Started.

[bradfitz]

Comment 8:

This issue was updated by revision 055b588.

R=golang-codereviews, dave, iant
CC=golang-codereviews
https://golang.org/cl/53350043

[bradfitz]

Comment 9:

Sent https://golang.org/cl/53470043 after looking at this with Ian.
If we're going to have a 64-bit Go syscall.Lock_t type for all POSIX platforms, it would
be nice to have a syscall number name that we could use to use it.  Unfortunately, the
correct syscall name varies by system... either FCNTL or FCNTL64.  So we should have a
func or method to do a fcntl lock.
The CL above adds a method to Lock_t.

[bradfitz]

Comment 10:

This issue was closed by revision 5d3033c.

Status changed to Fixed.

[bradfitz]

Comment 11:

This issue was updated by revision 367ad45.

R=rsc
CC=golang-codereviews
https://golang.org/cl/55370043