Re: [LTP] [PATCH 1/2] poll: add basic POLLHUP semantics test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Jinseok Kim <always.starving0@gmail.com>
> ---
>  testcases/kernel/syscalls/poll/poll03.c | 60 +++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/poll/poll03.c
> 
> diff --git a/testcases/kernel/syscalls/poll/poll03.c b/testcases/kernel/syscalls/poll/poll03.c
> new file mode 100644
> index 000000000..01189eb70
> --- /dev/null
> +++ b/testcases/kernel/syscalls/poll/poll03.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 Jinseok Kim <always.starving0@gmail.com>
> + */
> +
> +/*

This should be documentation comment which starts with /*\

> + * Check that poll() reports POLLHUP on a pipe read end
> + * after the write end has been closed.
> + */
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/poll.h>
> +
> +#include "tst_test.h"
> +
> +static int fds[2];
> +
> +void verify_pollhup(void)
> +{
> +	struct pollfd pfd = {
> +		.fd = fds[0], .events = POLLIN,
> +	};
> +
> +	SAFE_CLOSE(fds[1]);
> +	fds[1] = -1;

This is not needed, the SAFE_CLOSE() already sets the fd to -1.

> +	TEST(poll(&pfd, 1, -1));
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TTERRNO, "poll() failed");
> +		return;
> +	}

Here we should also check that we got correct return value. We expect to
get one event so:

	if (TST_RES != 1) {
		tst_res(TFAIL, "Unexpected poll() return value %i", TST_RET);
		return;
	}

> +	if (!(pfd.revents & POLLHUP)) {
> +		tst_res(TFAIL, "poll() did not report POLLHUP");
> +		return;
> +	}

Here as well, if we do not expect other bits set in the revents we
should also check that they are not set, something as:

	TST_EXP_EXPR(pfd.revents & POLLHUP);
	TST_EXP_EXPR((pfd.revents & ~POLLHUP) == 0);

> +	tst_res(TPASS, "poll() reported POLLHUP");
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_PIPE(fds);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fds[0] >= 0)
> +		SAFE_CLOSE(fds[0]);
> +
> +	if (fds[1] >= 0)
> +		SAFE_CLOSE(fds[1]);

These ifs does not work. You either need to initialize the fds to -1 or
check just fds[0] > 0 here. As it is you will close stdin if SAFE_PIPE()
fails.

> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = verify_pollhup,
> +};
> --
> 2.43.0
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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