Re: [LTP] syscalls: Add epoll_create03

[linuxtestproject.agent@gmail.com]

Hi Cyril,

On Thu, 09 Jul 2026 10:53:45 +0200, Cyril Hrubis wrote:
> syscalls: Add epoll_create03

--- [PATCH 1/9] ---

> /*\
>  * Verify that :manpage:epoll_create() fails with EMFILE when the per-process
>  * limit on the number of open file descriptors has been reached.
>  */

The :manpage: role is missing backticks and the section number. Should
be :manpage:`epoll_create(2)`.

--- [PATCH 2/9] ---

> /*\
>  * Verify that :manpage:epoll_wait() fails with EINTR when a signal arrives
>  * while waiting for events.
>  *
>  * [Algorithm]
>  *
>  * - Create an epoll instance
>  * - Fork a child that waits in the :manpage:epoll_wait(), parents sends
>  *   SIGUSR1 to interrupt it.
>  * - Verify that :manpage:epoll_wait() returns -1 with errno set to EINTR.
>  */

Three occurrences of :manpage:epoll_wait() are missing backticks and
the section number. Each should be :manpage:`epoll_wait(2)`.

"parents sends" should be "parent sends".

> 	TST_EXP_FAIL(epoll_wait(efd, &ev, 1, -1), EINTR,
> 		     "epoll_wait() interrupted by signal");

TST_EXP_FAIL is for syscalls that return 0 on success. epoll_wait()
returns a non-negative event count on success, so TST_EXP_FAIL2 is
the appropriate macro here.

In this particular setup it makes no practical difference (no fds
registered, infinite timeout), but using the wrong variant produces a
misleading error message if the expectation is ever violated.

--- [PATCH 3/9] ---

> /*\
>  * Verify that :manpage:epoll_wait() works with nested epoll instances up to
>  * the maximum nesting depth of 5 allowed by the kernel (EP_MAX_NESTS).
>  *
>  * [Algorithm]
>  *
>  * ...
>  * - Call :manpage:epoll_wait() on the outermost epoll instance and verify it

Two occurrences of :manpage:epoll_wait() are missing backticks and the
section number. Should be :manpage:`epoll_wait(2)`.

--- [PATCH 4/9] ---

> /*\
>  * Verify :manpage:epoll_ctl() EPOLL_CTL_ADD behavior across all file
>  * descriptor types.

The :manpage: role is missing backticks and the section number. Should
be :manpage:`epoll_ctl(2)`.

--- [PATCH 5/9] ---

> /*\
>  * Verify that :manpage:epoll_wait() with EPOLLPRI detects changes to
>  * /proc/mounts.

The :manpage: role is missing backticks and the section number. Should
be :manpage:`epoll_wait(2)`.

--- [PATCH 6/9] ---

> /*\
>  * Verify that :manpage:epoll_wait() reports EPOLLHUP when the write end of a
>  * pipe is closed while a child process is blocked in epoll_wait.
>  *
>  * [Algorithm]
>  *
>  * ...
>  * - The child's :manpage:epoll_wait() should return with EPOLLHUP.

Two occurrences of :manpage:epoll_wait() are missing backticks and the
section number. Should be :manpage:`epoll_wait(2)`.

--- [PATCH 7/9] ---

> 	rval = poll(fds, nfds, timeout);
> 	if (rval == -1) {
> 		tst_brk_(file, lineno, TBROK | TERRNO,
> 			"poll(%p, %li, %i) failed", fds, (long)nfds, timeout);
> 	} else if (rval < -1) {
> 		tst_brk_(file, lineno, TBROK | TERRNO,
> 			"Invalid poll(%p, %li, %i) return value %i",
> 			fds, (long)nfds, timeout, rval);
> 	}

poll() returns -1 on error or >= 0 on success; it cannot return a
value less than -1. The else-if branch is dead code.

The nearby safe_statvfs() uses "else if (rval)" to catch unexpected
non-zero returns for a function that is 0 on success. poll() returns
0 (timeout) and positive values (events ready) as normal outcomes, so
no such secondary check applies here. The body should just have the
rval == -1 guard and return rval.

> int safe_poll(const char *const file, const int lineno, struct pollfd *fds,
> 		nfds_t nfds, int timeout);
> #define SAFE_POLL(fds, nfds, timeout) \
> 	safe_poll(__FILE__, __LINE__, (fds), (nfds), (timeout))

New public functions in include/ must be documented with kernel-doc
syntax. A /** ... */ block covering the parameters and return value
is required.

--- [PATCH 8/9] ---

> /*\
>  * Verify that :manpage:epoll_wait() reports EPOLLERR when the read end of a
>  * pipe is closed while a child process is blocked in epoll_wait on the write
>  * end.
>  *
>  * [Algorithm]
>  *
>  * ...
>  * - The child's :manpage:epoll_wait() should return with EPOLLERR set.

Two occurrences of :manpage:epoll_wait() are missing backticks and the
section number. Should be :manpage:`epoll_wait(2)`.

--- [PATCH 9/9] ---

> Now that the epoll coverate is greater than it ever was we can remove

"coverate" should be "coverage".

> Fixes: https://github.com/linux-test-project/ltp/issues/860

The Fixes: tag must reference a commit hash in the local git history,
not a URL. For a GitHub issue reference use Closes: instead.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp