Re: [lttng-dev] [PATCH lttng-tools 2/2] Tests: select_poll_epoll: Add support for _time64

["Jérémie Galarneau via lttng-dev" <lttng-dev@lists.lttng.org>]

On Fri, Oct 07, 2022 at 10:39:18AM +1000, Alistair Francis via lttng-dev wrote:

Hi Alistair,

The first patch is good, I'll merge it in master.
Some comments on this patch follow.

> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Add support for the  64-bit time_t syscalls SYS_ppoll_time64
> and SYS_pselect6_time64.
> 
> These are the syscalls that exist 32-bit platforms since the 5.1 kernel.
> 32-bit platforms with a 64-bit time_t only have these and don't have the
> original syscalls (such as 32-bit RISC-V).
> 
> Fixes: https://github.com/lttng/lttng-tools/pull/162
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  tests/regression/kernel/select_poll_epoll.cpp | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/tests/regression/kernel/select_poll_epoll.cpp b/tests/regression/kernel/select_poll_epoll.cpp
> index c0b688217..4a6d394f4 100644
> --- a/tests/regression/kernel/select_poll_epoll.cpp
> +++ b/tests/regression/kernel/select_poll_epoll.cpp
> @@ -5,6 +5,7 @@
>   *
>   */
>  
> +#include <errno.h>
>  #include <fcntl.h>
>  #include <limits.h>
>  #include <poll.h>
> @@ -456,8 +457,22 @@ void ppoll_fds_buffer_overflow(
>  	ufds[0].fd = wait_fd;
>  	ufds[0].events = POLLIN|POLLPRI;
>  
> +#ifdef SYS_ppoll_time64
> +	/*
> +	 * As there is no timeout value, we don't convert to/from
> +	 * 64/32-bit time_t.
> +	 */
> +	ret = syscall(SYS_ppoll_time64, ufds, 100, NULL, NULL);
> +	if (ret == 0 || errno != ENOSYS) {
> +		goto ppoll_fds_buffer_overflow_done;
> +	}
> +#endif
> +

This results in the following warning when building for an architecture
that doesn't have SYS_ppoll_time64 defined:

label ‘ppoll_fds_buffer_overflow_done’ defined but not used [-Wunused-label]
ppoll_fds_buffer_overflow_done:
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
select_poll_epoll.cpp: In function ‘void ppoll_fds_ulong_max(FILE*)’:


Also, it is my understanding that both syscalls can be available on some
platforms. In that case, it would make sense to add them as separate
tests and skip tests that target non-existant syscalls.

Thanks!
Jérémie


> +#ifdef SYS_ppoll
>  	ret = syscall(SYS_ppoll, ufds, 100, NULL, NULL);
> +#endif
>  
> +ppoll_fds_buffer_overflow_done:
>  	if (ret < 0) {
>  		PERROR("ppoll");
>  	} else if (ret > 0) {
> @@ -483,7 +498,22 @@ void ppoll_fds_ulong_max(FILE *validation_output_file __attribute__((unused)))
>  	ufds[0].fd = wait_fd;
>  	ufds[0].events = POLLIN|POLLPRI;
>  
> +#ifdef SYS_ppoll_time64
> +	/*
> +	 * As there is no timeout value, we don't convert to/from
> +	 * 64/32-bit time_t.
> +	 */
> +	ret = syscall(SYS_ppoll_time64, ufds, ULONG_MAX, NULL, NULL);
> +	if (ret == 0 || errno != ENOSYS) {
> +		goto ppoll_fds_ulong_max_done;
> +	}
> +#endif
> +
> +#ifdef SYS_ppoll
>  	ret = syscall(SYS_ppoll, ufds, ULONG_MAX, NULL, NULL);
> +#endif
> +
> +ppoll_fds_ulong_max_done:
>  	if (ret < 0) {
>  		/* Expected error. */
>  	} else if (ret > 0) {
> @@ -524,7 +554,18 @@ void pselect_invalid_fd(FILE *validation_output_file __attribute__((unused)))
>  	FD_ZERO(&rfds);
>  	FD_SET(fd, &rfds);
>  
> +#ifdef SYS_pselect6_time64
> +	ret = syscall(SYS_pselect6_time64, fd + 1, &rfds, NULL, NULL, NULL, NULL);
> +	if (ret == 0 || errno != ENOSYS) {
> +		goto pselect_invalid_fd_done;
> +	}
> +#endif
> +
> +#ifdef SYS_pselect6
>  	ret = syscall(SYS_pselect6, fd + 1, &rfds, NULL, NULL, NULL, NULL);
> +#endif
> +
> +pselect_invalid_fd_done:
>  	if (ret == -1) {
>  		/* Expected error. */
>  	} else if (ret) {
> @@ -553,8 +594,20 @@ void pselect_invalid_pointer(
>  	FD_ZERO(&rfds);
>  	FD_SET(wait_fd, &rfds);
>  
> +#ifdef SYS_pselect6_time64
> +	ret = syscall(SYS_pselect6_time64, 1, &rfds, (fd_set *) invalid, NULL, NULL,
> +			NULL);
> +	if (ret == 0 || errno != ENOSYS) {
> +		goto pselect_invalid_pointer_done;
> +	}
> +#endif
> +
> +#ifdef SYS_pselect6
>  	ret = syscall(SYS_pselect6, 1, &rfds, (fd_set *) invalid, NULL, NULL,
>  			NULL);
> +#endif
> +
> +pselect_invalid_pointer_done:
>  	if (ret == -1) {
>  		/* Expected error. */
>  	} else if (ret) {
> -- 
> 2.37.3
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev