On 07.02.2019 14:52, Michał Górny wrote:
> Fix the kernel pty driver to report closed slave via master's kevent
> EVFILT_READ. This behavior matches the behavior for pipes, is
> consistent with how FreeBSD implements it and is relied upon by LLDB's
> main loop implementation.
> ---
> sys/kern/tty_pty.c | 4 +
> tests/kernel/kqueue/read/Makefile | 2 +
> tests/kernel/kqueue/read/t_pty_closed_slave.c | 94 +++++++++++++++++++
> 3 files changed, 100 insertions(+)
> create mode 100644 tests/kernel/kqueue/read/t_pty_closed_slave.c
>
> diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c
> index 1d60596124d3..94270381b5ab 100644
> --- a/sys/kern/tty_pty.c
> +++ b/sys/kern/tty_pty.c
> @@ -938,6 +938,10 @@ filt_ptcread(struct knote *kn, long hint)
> ((pti->pt_flags & PF_UCNTL) && pti->pt_ucntl))
> kn->kn_data++;
> }
> + if (!ISSET(tp->t_state, TS_CARR_ON)) {
> + kn->kn_flags |= EV_EOF;
> + canread = 1;
> + }
>
> if ((hint & NOTE_SUBMIT) == 0) {
> mutex_spin_exit(&tty_lock);
> diff --git a/tests/kernel/kqueue/read/Makefile b/tests/kernel/kqueue/read/Makefile
> index 510ed45c486c..d1a321aeb003 100644
> --- a/tests/kernel/kqueue/read/Makefile
> +++ b/tests/kernel/kqueue/read/Makefile
> @@ -10,8 +10,10 @@ TESTS_C= t_fifo
> TESTS_C+= t_file
> TESTS_C+= t_file2
> TESTS_C+= t_pipe
> +TESTS_C+= t_pty_closed_slave
> TESTS_C+= t_ttypty
Can we merge the test into t_ttypty? It seems to be a test for the same
functionality. New test could be named pty_closed_slave.
This way we will also reduce entries in distrib sets that still would
need to be updated with this patch.
>
> LDADD.t_ttypty= -lutil
> +LDADD.t_pty_closed_slave=-lutil
>
> .include <bsd.test.mk>
> diff --git a/tests/kernel/kqueue/read/t_pty_closed_slave.c b/tests/kernel/kqueue/read/t_pty_closed_slave.c
> new file mode 100644
> index 000000000000..1972103a2085
> --- /dev/null
> +++ b/tests/kernel/kqueue/read/t_pty_closed_slave.c
> @@ -0,0 +1,94 @@
> +/* $NetBSD$ */
> +
> +/*-
> + * Copyright (c) 2019 The NetBSD Foundation, Inc.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
> + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
> + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <sys/cdefs.h>
> +__COPYRIGHT("@(#) Copyright (c) 2019\
> + The NetBSD Foundation, inc. All rights reserved.");
> +__RCSID("$NetBSD$");
> +
> +#include <sys/event.h>
> +#include <sys/time.h>
> +
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <termios.h>
> +#include <unistd.h>
> +#include <util.h>
> +
> +#include <atf-c.h>
> +
> +#include "h_macros.h"
> +
> +ATF_TC(kevent);
> +ATF_TC_HEAD(kevent, tc)
> +{
> + atf_tc_set_md_var(tc, "descr",
> + "Checks EVFILT_READ reporting for slave tty being closed");
> +}
> +ATF_TC_BODY(kevent, tc)
> +{
> + char slavetty[1024];
> + struct kevent event[1];
> + int amaster, aslave;
> + int kq, n;
> +#if 0
> + int fl;
> +#endif
Is this fl needed here? Can we drop it?
> + struct timespec timeout = {5, 0};
> +
> + RL(openpty(&amaster, &aslave, slavetty, NULL, NULL));
> +
> + (void)printf("tty: openpty master %d slave %d tty '%s'\n",
> + amaster, aslave, slavetty);
> +
> + RL(kq = kqueue());
> +
> + EV_SET(&event[0], amaster, EVFILT_READ, EV_ADD|EV_ENABLE, 0, 0, 0);
> + RL(kevent(kq, event, 1, NULL, 0, NULL));
> +
> + RL(close(aslave));
> +
> + RL(n = kevent(kq, NULL, 0, event, 1, &timeout));
> +
> + (void)printf("kevent num %d filt %d flags: %#x, fflags: %#x, "
> + "data: %" PRId64 "\n", n, event[0].filter, event[0].flags,
> + event[0].fflags, event[0].data);
> +
> + ATF_REQUIRE_EQ(n, 1);
> + ATF_REQUIRE_EQ(event[0].filter, EVFILT_READ);
> + ATF_REQUIRE_EQ(event[0].flags & EV_EOF, EV_EOF);
> +
> + (void)printf("tty: successful end\n");
> +}
> +
> +ATF_TP_ADD_TCS(tp)
> +{
> + ATF_TP_ADD_TC(tp, kevent);
> +
> + return atf_no_error();
> +}
>
Attachment:
signature.asc
Description: OpenPGP digital signature