Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src - Add a new EVFILT_WRITE test case for FIFOs that correctly ...



details:   https://anonhg.NetBSD.org/src/rev/8a2398c5becf
branches:  trunk
changeset: 1023924:8a2398c5becf
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Sat Oct 02 18:39:15 2021 +0000

description:
- Add a new EVFILT_WRITE test case for FIFOs that correctly validates
  the writability thresholds.
- Fix a bug in fifo_kqfilter() exposed by the new test case; in the
  EVFILT_WRITE case, we were attaching the wrong end of the socket
  pair to the knote!
- In filt_fiforead(), use ">= so->so_rcv.sb_lowat" rather than "> 0"
  for consistency with fifo_poll().  NFC.

diffstat:

 sys/miscfs/fifofs/fifo_vnops.c     |    9 +-
 tests/kernel/kqueue/write/t_fifo.c |  122 +++++++++++++++++++++++++-----------
 2 files changed, 90 insertions(+), 41 deletions(-)

diffs (212 lines):

diff -r d76c91b2b525 -r 8a2398c5becf sys/miscfs/fifofs/fifo_vnops.c
--- a/sys/miscfs/fifofs/fifo_vnops.c    Sat Oct 02 18:21:05 2021 +0000
+++ b/sys/miscfs/fifofs/fifo_vnops.c    Sat Oct 02 18:39:15 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fifo_vnops.c,v 1.89 2021/10/02 17:37:21 thorpej Exp $  */
+/*     $NetBSD: fifo_vnops.c,v 1.90 2021/10/02 18:39:15 thorpej Exp $  */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.89 2021/10/02 17:37:21 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fifo_vnops.c,v 1.90 2021/10/02 18:39:15 thorpej Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -612,7 +612,7 @@
                rv = 1;
        } else {
                kn->kn_flags &= ~EV_EOF;
-               rv = (kn->kn_data > 0);
+               rv = (kn->kn_data >= so->so_rcv.sb_lowat);
        }
        if (hint != NOTE_SUBMIT)
                sounlock(so);
@@ -678,13 +678,14 @@
        struct socket   *so;
        struct sockbuf  *sb;
 
-       so = (struct socket *)ap->a_vp->v_fifoinfo->fi_readsock;
        switch (ap->a_kn->kn_filter) {
        case EVFILT_READ:
+               so = (struct socket *)ap->a_vp->v_fifoinfo->fi_readsock;
                ap->a_kn->kn_fop = &fiforead_filtops;
                sb = &so->so_rcv;
                break;
        case EVFILT_WRITE:
+               so = (struct socket *)ap->a_vp->v_fifoinfo->fi_writesock;
                ap->a_kn->kn_fop = &fifowrite_filtops;
                sb = &so->so_snd;
                break;
diff -r d76c91b2b525 -r 8a2398c5becf tests/kernel/kqueue/write/t_fifo.c
--- a/tests/kernel/kqueue/write/t_fifo.c        Sat Oct 02 18:21:05 2021 +0000
+++ b/tests/kernel/kqueue/write/t_fifo.c        Sat Oct 02 18:39:15 2021 +0000
@@ -1,11 +1,11 @@
-/* $NetBSD: t_fifo.c,v 1.4 2017/01/13 21:30:41 christos Exp $ */
+/* $NetBSD: t_fifo.c,v 1.5 2021/10/02 18:39:15 thorpej Exp $ */
 
 /*-
- * Copyright (c) 2002, 2008 The NetBSD Foundation, Inc.
+ * Copyright (c) 2021 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
- * by Luke Mewburn and Jaromir Dolecek.
+ * by Jason R. Thorpe.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -30,68 +30,116 @@
  */
 
 #include <sys/cdefs.h>
-__COPYRIGHT("@(#) Copyright (c) 2008\
+__COPYRIGHT("@(#) Copyright (c) 2021\
  The NetBSD Foundation, inc. All rights reserved.");
-__RCSID("$NetBSD: t_fifo.c,v 1.4 2017/01/13 21:30:41 christos Exp $");
+__RCSID("$NetBSD: t_fifo.c,v 1.5 2021/10/02 18:39:15 thorpej Exp $");
 
 #include <sys/event.h>
 #include <sys/stat.h>
-#include <sys/wait.h>
 
+#include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <inttypes.h>
 
 #include <atf-c.h>
 
-#include "h_macros.h"
+static const char      fifo_path[] = "fifo";
 
-#define FIFONAME "fifo"
+static void
+fifo_support(void)
+{
+       errno = 0;
+       if (mkfifo(fifo_path, 0600) == 0) {
+               ATF_REQUIRE(unlink(fifo_path) == 0);
+               return;
+       }
 
-ATF_TC(fifo);
+       if (errno == EOPNOTSUPP) {
+               atf_tc_skip("the kernel does not support FIFOs");
+       } else {
+               atf_tc_fail("mkfifo(2) failed");
+       }
+}
+
+ATF_TC_WITH_CLEANUP(fifo);
 ATF_TC_HEAD(fifo, tc)
 {
-       atf_tc_set_md_var(tc, "descr", "Checks EVFILT_WRITE for fifo");
+       atf_tc_set_md_var(tc, "descr", "Checks EVFILT_WRITE on fifo");
 }
 ATF_TC_BODY(fifo, tc)
 {
-       char buffer[128];
+       const struct timespec to = { 0, 0 };
        struct kevent event[1];
-       pid_t child;
-       int kq, n, fd, status;
+       char *buf;
+       int rfd, wfd, kq;
+       long pipe_buf;
 
-       RL(mkfifo(FIFONAME, 0644));
-       RL(fd = open(FIFONAME, O_RDWR, 0644));
-       RL(kq = kqueue());
+       fifo_support();
+
+       ATF_REQUIRE(mkfifo(fifo_path, 0600) == 0);
+       ATF_REQUIRE((rfd = open(fifo_path, O_RDONLY | O_NONBLOCK)) >= 0);
+       ATF_REQUIRE((wfd = open(fifo_path, O_WRONLY | O_NONBLOCK)) >= 0);
+       ATF_REQUIRE((kq = kqueue()) >= 0);
 
-       /* spawn child reader */
-       RL(child = fork());
-       if (child == 0) {
-               int sz = read(fd, buffer, 128);
-               if (sz > 0)
-                       (void)printf("fifo: child read '%.*s'\n", sz, buffer);
-               _exit(sz <= 0);
-       }
+       /* Get the maximum atomic pipe write size. */
+       pipe_buf = fpathconf(wfd, _PC_PIPE_BUF);
+       ATF_REQUIRE(pipe_buf > 1);
+
+       buf = malloc(pipe_buf);
+       ATF_REQUIRE(buf != NULL);
+
+       EV_SET(&event[0], wfd, EVFILT_WRITE, EV_ADD|EV_ENABLE, 0, 0, 0);
+       ATF_REQUIRE(kevent(kq, event, 1, NULL, 0, NULL) == 0);
+
+       /* We expect the FIFO to be writable. */
+       ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 1);
+       ATF_REQUIRE(event[0].ident == (uintptr_t)wfd);
+       ATF_REQUIRE(event[0].filter == EVFILT_WRITE);
 
-       EV_SET(&event[0], fd, EVFILT_WRITE, EV_ADD|EV_ENABLE, 0, 0, 0);
-       RL(kevent(kq, event, 1, NULL, 0, NULL));
+       /*
+        * Write data into the FIFO until it is full, which is
+        * defined as insufficient buffer space to hold the
+        * maximum atomic pipe write size.
+        */
+       while (write(wfd, buf, pipe_buf) != -1) {
+               continue;
+       }
+       ATF_REQUIRE(errno == EAGAIN);
 
-       (void)memset(event, 0, sizeof(event));
-       RL(n = kevent(kq, NULL, 0, event, 1, NULL));
+       /* We expect the FIFO to no longer be writable. */
+       ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 0);
+
+       /* Read a single byte of data from the FIFO. */
+       ATF_REQUIRE(read(rfd, buf, 1) == 1);
 
-       (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(event[0].filter, EVFILT_WRITE);
+       /*
+        * Because we have read only a single byte out, there will
+        * be insufficient space for a pipe_buf-sized message, so
+        * the FIFO should still not be writable.
+        */
+       ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 0);
 
-       RL(write(fd, "foo", 3));
-       (void)printf("fifo: wrote 'foo'\n");
-       RL(close(fd));
+       /*
+        * Now read enough so that exactly pipe_buf space should be
+        * available.  The FIFO should be writable after that.
+        */
+       ATF_REQUIRE(read(rfd, buf, pipe_buf - 1) == pipe_buf - 1);
+       ATF_REQUIRE(kevent(kq, NULL, 0, event, 1, &to) == 1);
+       ATF_REQUIRE(event[0].ident == (uintptr_t)wfd);
+       ATF_REQUIRE(event[0].filter == EVFILT_WRITE);
 
-       (void)waitpid(child, &status, 0);
+       (void)close(wfd);
+       (void)close(rfd);
+       (void)close(kq);
+}
+
+ATF_TC_CLEANUP(fifo, tc)
+{
+       (void)unlink(fifo_path);
 }
 
 ATF_TP_ADD_TCS(tp)



Home | Main Index | Thread Index | Old Index