aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKyle Evans <kevans@FreeBSD.org>2024-01-16 02:55:59 +0000
committerKyle Evans <kevans@FreeBSD.org>2024-01-16 02:55:59 +0000
commit30189156d325fbcc9d1997d791daedc9fa3bed20 (patch)
tree25398a02c260f9b358079db40cb45ab49240c282
parent522083ffbd1ab9b485861750e889d606dc75ed0a (diff)
downloadsrc-30189156d325fbcc9d1997d791daedc9fa3bed20.tar.gz
src-30189156d325fbcc9d1997d791daedc9fa3bed20.zip
kern: pts: do not special case closed slave side
This would previously return 1 if the slave side of the pts was closed to force an application to read() from it and observe the EOF, but it's not clear why and this is inconsistent both with how we handle devices with similar mechanics (like pipes) and also with other kernels, such as OpenBSD/NetBSD and Linux. PR: 239604 Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D43457
-rw-r--r--sys/kern/tty_pts.c7
-rw-r--r--tests/sys/kern/Makefile3
-rw-r--r--tests/sys/kern/tty_pts.c64
3 files changed, 68 insertions, 6 deletions
diff --git a/sys/kern/tty_pts.c b/sys/kern/tty_pts.c
index f79c9cc3e979..31122d422a28 100644
--- a/sys/kern/tty_pts.c
+++ b/sys/kern/tty_pts.c
@@ -271,12 +271,7 @@ ptsdev_ioctl(struct file *fp, u_long cmd, void *data,
return (0);
case FIONREAD:
tty_lock(tp);
- if (psc->pts_flags & PTS_FINISHED) {
- /* Force read() to be called. */
- *(int *)data = 1;
- } else {
- *(int *)data = ttydisc_getc_poll(tp);
- }
+ *(int *)data = ttydisc_getc_poll(tp);
tty_unlock(tp);
return (0);
case FIODGNAME:
diff --git a/tests/sys/kern/Makefile b/tests/sys/kern/Makefile
index 143c13297585..7b5d87ff30ac 100644
--- a/tests/sys/kern/Makefile
+++ b/tests/sys/kern/Makefile
@@ -40,6 +40,7 @@ ATF_TESTS_C+= subr_physmem_test
PLAIN_TESTS_C+= subr_unit_test
ATF_TESTS_C+= sysctl_kern_proc
ATF_TESTS_C+= sys_getrandom
+ATF_TESTS_C+= tty_pts
ATF_TESTS_C+= unix_dgram
ATF_TESTS_C+= unix_passfd_dgram
TEST_METADATA.unix_passfd_dgram+= is_exclusive="true"
@@ -89,6 +90,8 @@ NETBSD_ATF_TESTS_C+= sysv_test
CFLAGS.mqueue_test+= -I${SRCTOP}/tests
LIBADD.mqueue_test+= rt
+LIBADD.tty_pts+= atf_c util
+
ATF_TESTS_C+= libkern_crc32
SRCS.libkern_crc32+= libkern_crc32.c
.PATH: ${SRCTOP}/sys/libkern
diff --git a/tests/sys/kern/tty_pts.c b/tests/sys/kern/tty_pts.c
new file mode 100644
index 000000000000..241cb085a5a7
--- /dev/null
+++ b/tests/sys/kern/tty_pts.c
@@ -0,0 +1,64 @@
+/*-
+ * Copyright (c) 2024 Kyle Evans <kevans@FreeBSD.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <termios.h>
+
+#include <atf-c.h>
+#include <libutil.h>
+
+/* Just a little more concise. */
+#define newpty(masterp, slavep) openpty((masterp), (slavep), NULL, NULL, NULL)
+
+ATF_TC_WITHOUT_HEAD(fionread);
+ATF_TC_BODY(fionread, tc)
+{
+ char rbuf[32];
+ char buf[] = "Hello";
+ int master, slave;
+ int bytes;
+
+ ATF_REQUIRE_EQ(0, newpty(&master, &slave));
+
+ /* Should be empty to begin with. */
+ ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes));
+ ATF_REQUIRE_EQ(0, bytes);
+
+ ATF_REQUIRE_EQ(sizeof(buf) - 1, write(slave, buf, sizeof(buf) - 1));
+ ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes));
+ ATF_REQUIRE_EQ(sizeof(buf) - 1, bytes);
+
+ /* Drain what we have available, should result in 0 bytes again. */
+ ATF_REQUIRE_EQ(sizeof(buf) - 1, read(master, rbuf, sizeof(rbuf)));
+ ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes));
+ ATF_REQUIRE_EQ(0, bytes);
+
+ /*
+ * Write once more, then close the slave side with data still in the
+ * buffer.
+ */
+ ATF_REQUIRE_EQ(sizeof(buf) - 1, write(slave, buf, sizeof(buf) - 1));
+ ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes));
+ ATF_REQUIRE_EQ(sizeof(buf) - 1, bytes);
+
+ ATF_REQUIRE_EQ(0, close(slave));
+
+ /*
+ * The tty's output queue is discarded upon close, so we shouldn't have
+ * anything else to read().
+ */
+ ATF_REQUIRE_EQ(0, ioctl(master, FIONREAD, &bytes));
+ ATF_REQUIRE_EQ(0, bytes);
+ ATF_REQUIRE_EQ(0, read(master, rbuf, sizeof(rbuf)));
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+ ATF_TP_ADD_TC(tp, fionread);
+ return (atf_no_error());
+}