Re: nonnull on epoll_wait(2) syscall wrappers?

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 19/05/23 19:50, Alejandro Colomar wrote:
> Hi Adhemerval,
> 
> I was checking the epoll_pwait2(2) man page to see if it needs some
> updating for the wrapper you added recently.  It seems all's good.
> 
> <https://sourceware.org/bugzilla/show_bug.cgi?id=27359>
> 
> However, I noticed that there's a difference between the current
> manual page and the glibc wrappers for the 3 related functions:
> 
> 
> $ grepc -x /sys/epoll.h$ epoll_wait /usr/include/
> /usr/include/x86_64-linux-gnu/sys/epoll.h:124:
> extern int epoll_wait (int __epfd, struct epoll_event *__events,
> 		       int __maxevents, int __timeout)
> 	__attr_access ((__write_only__, 2, 3));
> $ grepc -x /sys/epoll.h$ epoll_pwait /usr/include/
> /usr/include/x86_64-linux-gnu/sys/epoll.h:134:
> extern int epoll_pwait (int __epfd, struct epoll_event *__events,
> 			int __maxevents, int __timeout,
> 			const __sigset_t *__ss)
> 	__attr_access ((__write_only__, 2, 3));
> $ grepc -x /sys/epoll.h$ epoll_pwait2 /usr/include/
> /usr/include/x86_64-linux-gnu/sys/epoll.h:144:
> extern int epoll_pwait2 (int __epfd, struct epoll_event *__events,
> 			 int __maxevents, const struct timespec *__timeout,
> 			 const __sigset_t *__ss)
> 	__attr_access ((__write_only__, 2, 3));
> 
> 
> /usr/include/x86_64-linux-gnu/sys/epoll.h:157:
> #  define epoll_pwait2 __epoll_pwait2_time64
> 
> 
> The man page synopsis is:
> 
> 
> SYNOPSIS
>        #include <sys/epoll.h>
> 
>        int epoll_wait(int epfd, struct epoll_event *events,
>                       int maxevents, int timeout);
>        int epoll_pwait(int epfd, struct epoll_event *events,
>                       int maxevents, int timeout,
>                       const sigset_t *_Nullable sigmask);
>        int epoll_pwait2(int epfd, struct epoll_event *events,
>                       int maxevents, const struct timespec *_Nullable timeout,
>                       const sigset_t *_Nullable sigmask);
> 
> 
> I didn't use _Nullable in the events parameter, because I don't think
> it can be NULL.  Does it make any sense in any case having a NULL
> there?  Should we use nonnull in glibc?

It does make sense to mark the epoll_events argument as nonnull, since
either the kernel will return EINVAL for maxevents equal to 0 or
EFAULT for invalid events:

fs/eventpoll.c

2284  * Implement the event wait interface for the eventpoll file. It is the kernel
2285  * part of the user space epoll_wait(2).
2286  */
2287 static int do_epoll_wait(int epfd, struct epoll_event __user *events,
2288                          int maxevents, struct timespec64 *to)
2289 {
2290         int error;
2291         struct fd f;
2292         struct eventpoll *ep;
2293
2294         /* The maximum number of event must be greater than zero */
2295         if (maxevents <= 0 || maxevents > EP_MAX_EVENTS)
2296                 return -EINVAL;
2297
2298         /* Verify that the area passed by the user is writeable */
2299         if (!access_ok(events, maxevents * sizeof(struct epoll_event)))
2300                 return -EFAULT;