standards/49684: readlinkat(2) is not compliant

[Pierre Pronchery <khorben%defora.org@localhost>]

>Number:         49684
>Category:       standards
>Synopsis:       readlinkat(2) is not compliant
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    standards-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 21 23:15:00 +0000 2015
>Originator:     Pierre Pronchery
>Release:        NetBSD 7.0_BETA
>Organization:
The NetBSD Foundation
>Environment:
>Description:
There is a discrepancy between the manual page for readlinkat(2) and the
actual prototype for this system call as found in /usr/include/unistd.h.

The manual page says:
  ssize_t
  readlinkat(int fd, const char * restrict path, char * restrict buf,
      size_t bufsiz);

Whereas the header actually contains:
  int
  readlinkat(int fd, const char *, char *, size_t);

The issue is therefore with the return value, respectively typed ssize_t
and int. Somewhat interestingly, I noticed this because of a build issue
in a software project failing to recognize support of readlinkat(2) in
NetBSD, and then providing a conflicting prototype for it.

Last but not least, according to The Open Group Base Specifications
Issue 7, the prototype should be:
  ssize_t
  readlinkat(int fd, const char *restrict path, char *restrict buf,
      size_t bufsize);

Just like the manual says. I am afraid this may involve an ABI update in
NetBSD's libc, but I do not know for sure as of now.

A quick look at the functions surrounding readlinkat(2) in unistd.h:
- linkat() is fine
- renameat() is fine
- faccessat() is fine
- fchownat() apparently not in the specification online
- symlinkat() is fine
- unlinkat() is fine

Likewise readlink(2) looks good to me. My immediate knowledge does not
allow me to be sure whether the actual implementation in
sys/kern/vfs_syscalls.c is impacted at all. It seems to be that the
result might be truncated from sizeof(ssize_t) to sizeof(int) on some
platforms (like every 32-bits I know). But I will welcome a better
expertise here.

>How-To-Repeat:
Build wip/pkg15 1.4.99.11nb4 from pkgsrc on NetBSD 7.0_BETA or newer.
>Fix:
Probably involves modifying /usr/include/unistd.h as follows:

Index: include/unistd.h
===================================================================
RCS file: /cvsroot/src/include/unistd.h,v
retrieving revision 1.143
diff -p -u -r1.143 unistd.h
--- include/unistd.h	26 Sep 2014 19:28:03 -0000	1.143
+++ include/unistd.h	21 Feb 2015 22:41:29 -0000
@@ -308,7 +308,7 @@ int	linkat(int, const char *, int, const
 int	renameat(int, const char *, int, const char *);
 int	faccessat(int, const char *, int, int);
 int	fchownat(int, const char *, uid_t, gid_t, int);
-int	readlinkat(int, const char *, char *, size_t);
+ssize_t	readlinkat(int, const char *, char *, size_t);
 int	symlinkat(const char *, int, const char *);
 int	unlinkat(int, const char *, int);
 #endif

Then also syscalls.master:

Index: sys/kern/syscalls.master
===================================================================
RCS file: /cvsroot/src/sys/kern/syscalls.master,v
retrieving revision 1.271
diff -p -u -r1.271 syscalls.master
--- sys/kern/syscalls.master	10 Feb 2015 15:07:39 -0000	1.271
+++ sys/kern/syscalls.master	21 Feb 2015 22:59:07 -0000
@@ -919,7 +919,7 @@
 			    const struct timespec *tptr, int flag); }
 468	STD  RUMP	{ int|sys||openat(int fd, const char *path, \
 			    int oflags, ... mode_t mode); }
-469	STD  RUMP	{ int|sys||readlinkat(int fd, const char *path, \
+469	STD  RUMP	{ ssize_t|sys||readlinkat(int fd, const char *path, \
 			    char *buf, size_t bufsize); }
 470	STD  RUMP	{ int|sys||symlinkat(const char *path1, int fd, \
 			    const char *path2); }

(then of course re-generating everything related)

>Unformatted:
 Built on February 16th 2015, up to tickets #500 and #501 applied.
 (EdgeBSD's commit ID c2bd92a3)