x/sys/unix: InotifyRmWatch should take int, not uint32

What version of Go are you using (go version)?

$ go version
go1.10.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, https://github.com/golang/sys/blob/master/unix/zsyscall_linux_arm64.go still shows the following signature: InotifyRmWatch(fd int, watchdesc uint32).

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home//.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home//go"
GORACE=""
GOROOT="/usr/lib/go-1.10"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.10/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build762147821=/tmp/go-build -gno-record-gcc-switches"

What did you do?

wd,_ := unix.InotifyAddWatch(...) // wd is of type int
unix.InotifyRmWatch(..., wd) // function expects uint32 but wd is of type int

What did you expect to see?

According to man, inotify_rm_watch is defined as: int inotify_rm_watch(int fd, int wd). So I would expect InotifyRmWatch to take two regular ints.

What did you see instead?

Compiler complaining about type conversion (int -> uint32).

[FiloSottile]

The definition in syscall_linux.go dates back to 5 years ago, but it does look mismatched.

/cc owners @ianlancetaylor @bradfitz @tklauser

[bradfitz]

I guess it depends whether changing it would do more good than harm.

I suspect it'd break a number of people and the benefit of changing it seems low-ish.

[akemrir]

This looks like discrepancy. Refering to inotify man pages:
https://www.man7.org/linux/man-pages/man2/inotify_rm_watch.2.html

int inotify_rm_watch(int fd, int wd);

https://www.man7.org/linux/man-pages/man2/inotify_add_watch.2.html

int inotify_add_watch(int fd, const char *pathname, uint32_t mask);

wd and return values when adding watch in inotify are of the same types.

Go code is generated from: src/syscall/syscall_linux.go
Where we have:

...
//sys	InotifyAddWatch(fd int, pathname string, mask uint32) (watchdesc int, err error)
//sysnb	InotifyRmWatch(fd int, watchdesc uint32) (success int, err error)
...

uint32 is generally used for masks and modes

Even found in old articles it's always stated as int:
https://lwn.net/Articles/604686/

In linux kernel we can see:
any-linux-any/linux/inotify.h

/* For O_CLOEXEC and O_NONBLOCK */
#include <linux/fcntl.h>
#include <linux/types.h>

/*
 * struct inotify_event - structure read from the inotify device for each event
 *
 * When you are watching a directory, you will receive the filename for events
 * such as IN_CREATE, IN_DELETE, IN_OPEN, IN_CLOSE, ..., relative to the wd.
 */
struct inotify_event {
	__s32		wd;		/* watch descriptor */
	__u32		mask;		/* watch mask */
	__u32		cookie;		/* cookie to synchronize two events */
	__u32		len;		/* length (including nulls) of name */
	char		name[];	/* stub for possible name */
};

types.h
-> asm/types.h
-> asm-generic/types.h
-> asm-generic/int-ll64.h

showing

#ifndef __ASSEMBLY__
/*
 * __xx is ok: it doesn't pollute the POSIX namespace. Use these in the
 * header files exported to user space
 */

typedef __signed__ char __s8;
typedef unsigned char __u8;

typedef __signed__ short __s16;
typedef unsigned short __u16;

typedef __signed__ int __s32;
typedef unsigned int __u32;

[ianlancetaylor]

Perhaps we made a mistake, but changing this now would just break existing working code for no benefit.

[akemrir]

Ok, it would break existing working code, but this hangs for more than 5 years.
I've found a trace in api/go1.txt with same signature...

I would like to ask: when it could be fixed?

[illarion]

@ianlancetaylor Now we have versions, so maybe, there's a chance to have syscall/v2 package, with fixed discrepancies like this, without breaking backward compatibility ?