* [LTP] [PATCH] syscalls/pipe2_01: convert into new API
@ 2020-04-10 8:52 Yang Xu
2020-04-15 5:11 ` [LTP] [PATCH v2] " Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-10 8:52 UTC (permalink / raw)
To: ltp
Also, merge pipe2_02.c into this case and use SAFE_PIPE2 macro(it is in
review). I have a kernel doubt when using O_DIRECT flag, for read side,
it is ignore as below:
fs/pipe.c
create_pipe_files(...)
....
res[0] = alloc_file_clone(f, O_RDONLY | (flags & O_NONBLOCK),
&pipefifo_fops)
In internal, this O_DIRECT flag is used to set flag with buf as below:
if (is_packetized(filp)) {
buf->ops = &packet_pipe_buf_ops;
buf->flags = PIPE_BUF_FLAG_PACKET;
}
but we can set this flag in read side by fcntl with SETFL, GETFL command can
get this flag correctly. Do we should remove this filter for O_DIRECT flag in res[0]?
Or, I should skip this sub test when using O_DIRECT flag for read side?
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
runtest/syscalls | 1 -
testcases/kernel/syscalls/pipe2/.gitignore | 1 -
testcases/kernel/syscalls/pipe2/pipe2_01.c | 228 +++++----------------
testcases/kernel/syscalls/pipe2/pipe2_02.c | 175 ----------------
4 files changed, 52 insertions(+), 353 deletions(-)
delete mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 5607ae9a3..d3edc1126 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -919,7 +919,6 @@ pipe12 pipe12
pipe13 pipe13
pipe2_01 pipe2_01
-pipe2_02 pipe2_02
pipe2_03 pipe2_03
pivot_root01 pivot_root01
diff --git a/testcases/kernel/syscalls/pipe2/.gitignore b/testcases/kernel/syscalls/pipe2/.gitignore
index 01d980dba..e54774705 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1,3 +1,2 @@
/pipe2_01
-/pipe2_02
/pipe2_03
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_01.c b/testcases/kernel/syscalls/pipe2/pipe2_01.c
index 5f3ad8f0f..af86e4c35 100644
--- a/testcases/kernel/syscalls/pipe2/pipe2_01.c
+++ b/testcases/kernel/syscalls/pipe2/pipe2_01.c
@@ -1,186 +1,62 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_01.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich??s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ed8cae8ba01348bfd83333f4648dd807b04d7f08 */
-/* says: */
-/* This patch introduces the new syscall pipe2 which is like pipe but it also */
-/* takes an additional parameter which takes a flag value. This patch */
-/* implements the handling of O_CLOEXEC for the flag. I did not add support */
-/* for the new syscall for the architectures which have a special sys_pipe */
-/* implementation. I think the maintainers of those archs have the chance to */
-/* go with the unified implementation but that's up to them. */
-/* */
-/* The implementation introduces do_pipe_flags. I did that instead of */
-/* changing all callers of do_pipe because some of the callers are written in */
-/* assembler. I would probably screw up changing the assembly code. To avoid */
-/* breaking code do_pipe is now a small wrapper around do_pipe_flags. Once */
-/* all callers are changed over to do_pipe_flags the old do_pipe function can */
-/* be removed. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_01 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_01 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
+// SPDX-License-Identifier: GPL-2.0-or-late
+/*
+ * Copyright (c) Ulrich Drepper <drepper@redhat.com>
+ * Copyright (c) International Business Machines Corp., 2009
+ *
+ * Author: Ulrich Drepper <drepper@redhat.com>
+ *
+ * History:
+ * Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com>
+ * Ported to LTP - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com>
+ */
+#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_01";
-int testno;
-int TST_TOTAL = 1;
-
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
-
- tst_rmdir();
-
-}
-
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
+#include "tst_test.h"
+
+static int fds[2];
+
+static struct tcase {
+ int flags;
+ int check_cmd;
+ char *message;
+} tcases[] = {
+ {0, F_GETFD, "Test pipe2 with 0 flag"},
+ {O_CLOEXEC, F_GETFD, "Test pipe2 using O_CLOEXEC flag"},
+ {O_DIRECT, F_GETFL, "Test pipe2 using O_DIRECT flag"},
+ {O_NONBLOCK, F_GETFL, "Test pipe2 using O_NONBLOCK flag"},
+};
+
+static void cleanup(void)
{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
+ if (fds[0] > 0)
+ SAFE_CLOSE(fds[0]);
+ if (fds[1] > 1)
+ SAFE_CLOSE(fds[1]);
}
-int main(int argc, char *argv[])
+static void verify_pipe2(unsigned int n)
{
- int fd[2], i, coe;
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
+ struct tcase *tc = &tcases[n];
+ int get_flag = 0, i = 0;
+
+ tst_res(TINFO, "%s ", tc->message);
+ for (i = 0; i < 2; i++) {
+ SAFE_PIPE2(fds, tc->flags);
+ get_flag = SAFE_FCNTL(fds[i], tc->check_cmd);
+ if ((get_flag && tc->flags) || (tc->flags == get_flag))
+ tst_res(TPASS, "pipe2 fds[%d] gets expected flag(%d)", i, tc->flags);
+ else
+ tst_res(TFAIL, "pipe2 fds[%d] doesn't get expected flag(%d), get flag(%d)",
+ i, tc->flags, get_flag);
}
- setup();
+ cleanup();
+}
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fd, 0) != 0) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (coe & FD_CLOEXEC) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
+static struct tst_test test = {
+ .tcnt = ARRAY_SIZE(tcases),
+ .test = verify_pipe2,
+ .cleanup = cleanup,
+};
- if (ltp_syscall(__NR_pipe2, fd, O_CLOEXEC) != 0) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_CLOEXEC) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((coe & FD_CLOEXEC) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_CLOEXEC) does not set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
- tst_resm(TPASS, "pipe2(O_CLOEXEC) PASSED");
- cleanup();
- }
- }
- tst_exit();
-}
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c b/testcases/kernel/syscalls/pipe2/pipe2_02.c
deleted file mode 100644
index 39f02ff8c..000000000
--- a/testcases/kernel/syscalls/pipe2/pipe2_02.c
+++ /dev/null
@@ -1,175 +0,0 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_02.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich??s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=be61a86d7237dd80510615f38ae21d6e1e98660c */
-/* which says: */
-/* This patch adds O_NONBLOCK support to pipe2. It is minimally more involved*/
-/* than the patches for eventfd et.al but still trivial. The interfaces of */
-/* the create_write_pipe and create_read_pipe helper functions were changed */
-/* and the one other caller as well. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_02 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_02 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
-#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_02";
-int testno;
-int TST_TOTAL = 1;
-
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
-
- tst_rmdir();
-
-}
-
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
-{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
-}
-
-int main(int argc, char *argv[])
-{
- int fds[2], fl, i;
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
- }
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fds, 0) == -1) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (fl & O_NONBLOCK) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set non-blocking mode for fds[%d]",
- i);
- }
- close(fds[i]);
- }
-
- if (ltp_syscall(__NR_pipe2, fds, O_NONBLOCK) == -1) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_NONBLOCK) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((fl & O_NONBLOCK) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_NONBLOCK) does not set non-blocking mode for fds[%d]\n",
- i);
- }
- close(fds[i]);
- }
- tst_resm(TPASS, "pipe2(O_NONBLOCK) PASSED");
- cleanup();
- }
- }
- tst_exit();
-}
--
2.23.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH v2] syscalls/pipe2_01: convert into new API
2020-04-10 8:52 [LTP] [PATCH] syscalls/pipe2_01: convert into new API Yang Xu
@ 2020-04-15 5:11 ` Yang Xu
2020-04-16 7:28 ` [LTP] [PATCH v3 0/3] increase pipe2 coverage Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-15 5:11 UTC (permalink / raw)
To: ltp
Also, add O_DIRECT flag test. This flag was introduced since kernel 3.4.
[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9883035ae7ed
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v1->v2:
1. skip O_DIRECT flag test in read side and add some message
2. define O_DIRECT flag in lapi/fcntl.h to avoid undefined error
3. use SAFE_PIPE2 outside of for loop
include/lapi/fcntl.h | 4 +
runtest/syscalls | 1 -
testcases/kernel/syscalls/pipe2/.gitignore | 1 -
testcases/kernel/syscalls/pipe2/pipe2_01.c | 234 ++++++---------------
testcases/kernel/syscalls/pipe2/pipe2_02.c | 175 ---------------
5 files changed, 65 insertions(+), 350 deletions(-)
delete mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index 24aa46e1f..576a18daf 100644
--- a/include/lapi/fcntl.h
+++ b/include/lapi/fcntl.h
@@ -9,6 +9,10 @@
#include <fcntl.h>
#include <sys/socket.h>
+#ifndef O_DIRECT
+# define O_DIRECT 040000
+#endif
+
#ifndef O_CLOEXEC
# define O_CLOEXEC 02000000
#endif
diff --git a/runtest/syscalls b/runtest/syscalls
index 44254d7da..79b671d50 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -911,7 +911,6 @@ pipe12 pipe12
pipe13 pipe13
pipe2_01 pipe2_01
-pipe2_02 pipe2_02
pivot_root01 pivot_root01
diff --git a/testcases/kernel/syscalls/pipe2/.gitignore b/testcases/kernel/syscalls/pipe2/.gitignore
index cd38bb309..42350bbdc 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1,2 +1 @@
/pipe2_01
-/pipe2_02
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_01.c b/testcases/kernel/syscalls/pipe2/pipe2_01.c
index 5f3ad8f0f..86f3f18d6 100644
--- a/testcases/kernel/syscalls/pipe2/pipe2_01.c
+++ b/testcases/kernel/syscalls/pipe2/pipe2_01.c
@@ -1,186 +1,74 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_01.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich?s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ed8cae8ba01348bfd83333f4648dd807b04d7f08 */
-/* says: */
-/* This patch introduces the new syscall pipe2 which is like pipe but it also */
-/* takes an additional parameter which takes a flag value. This patch */
-/* implements the handling of O_CLOEXEC for the flag. I did not add support */
-/* for the new syscall for the architectures which have a special sys_pipe */
-/* implementation. I think the maintainers of those archs have the chance to */
-/* go with the unified implementation but that's up to them. */
-/* */
-/* The implementation introduces do_pipe_flags. I did that instead of */
-/* changing all callers of do_pipe because some of the callers are written in */
-/* assembler. I would probably screw up changing the assembly code. To avoid */
-/* breaking code do_pipe is now a small wrapper around do_pipe_flags. Once */
-/* all callers are changed over to do_pipe_flags the old do_pipe function can */
-/* be removed. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_01 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_01 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
+// SPDX-License-Identifier: GPL-2.0-or-late
+/*
+ * Copyright (c) Ulrich Drepper <drepper@redhat.com>
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ *
+ * Author: Ulrich Drepper <drepper@redhat.com>
+ *
+ * History:
+ * Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com>
+ * Ported to LTP - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com>
+ * Converted into new api - Apri 15 2020 - Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ */
+#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_01";
-int testno;
-int TST_TOTAL = 1;
+#include "tst_test.h"
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
+static int fds[2];
- tst_rmdir();
-
-}
+static struct tcase {
+ int flags;
+ int check_cmd;
+ int check_read_side;
+ char *message;
+} tcases[] = {
+ {0, F_GETFD, 1, "Test pipe2 with 0 flag"},
+ {O_CLOEXEC, F_GETFD, 1, "Test pipe2 using O_CLOEXEC flag"},
+ /*
+ * It may get EINVAL error on older kernel because this flag was
+ * introduced since kernel 3.4. We only test flag in write side
+ * because this flag was used to make pipe buffer marked with the
+ * PIPE_BUF_FLAG_PACKET flag. In read side, kernel also check buffer
+ * flag instead of O_DIRECT. So it make no sense to check this flag
+ * in fds[0].
+ */
+ {O_DIRECT, F_GETFL, 0, "Test pipe2 using O_DIRECT flag"},
+ {O_NONBLOCK, F_GETFL, 1, "Test pipe2 using O_NONBLOCK flag"},
+};
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
+static void cleanup(void)
{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
+ if (fds[0] > 0)
+ SAFE_CLOSE(fds[0]);
+ if (fds[1] > 1)
+ SAFE_CLOSE(fds[1]);
}
-int main(int argc, char *argv[])
+static void verify_pipe2(unsigned int n)
{
- int fd[2], i, coe;
- int lc;
+ struct tcase *tc = &tcases[n];
+ int get_flag = 0, i = 0;
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
+ tst_res(TINFO, "%s ", tc->message);
+ SAFE_PIPE2(fds, tc->flags);
+ for (i = 0; i < 2; i++) {
+ if (i == 0 && !tc->check_read_side)
+ continue;
+ get_flag = SAFE_FCNTL(fds[i], tc->check_cmd);
+ if ((get_flag && tc->flags) || (tc->flags == get_flag))
+ tst_res(TPASS, "pipe2 fds[%d] gets expected flag(%d)", i, tc->flags);
+ else
+ tst_res(TFAIL, "pipe2 fds[%d] doesn't get expected flag(%d), get flag(%d)",
+ i, tc->flags, get_flag);
}
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fd, 0) != 0) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (coe & FD_CLOEXEC) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
-
- if (ltp_syscall(__NR_pipe2, fd, O_CLOEXEC) != 0) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_CLOEXEC) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((coe & FD_CLOEXEC) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_CLOEXEC) does not set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
- tst_resm(TPASS, "pipe2(O_CLOEXEC) PASSED");
- cleanup();
- }
- }
- tst_exit();
+ cleanup();
}
+
+static struct tst_test test = {
+ .tcnt = ARRAY_SIZE(tcases),
+ .test = verify_pipe2,
+ .cleanup = cleanup,
+};
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c b/testcases/kernel/syscalls/pipe2/pipe2_02.c
deleted file mode 100644
index 39f02ff8c..000000000
--- a/testcases/kernel/syscalls/pipe2/pipe2_02.c
+++ /dev/null
@@ -1,175 +0,0 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_02.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich?s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=be61a86d7237dd80510615f38ae21d6e1e98660c */
-/* which says: */
-/* This patch adds O_NONBLOCK support to pipe2. It is minimally more involved*/
-/* than the patches for eventfd et.al but still trivial. The interfaces of */
-/* the create_write_pipe and create_read_pipe helper functions were changed */
-/* and the one other caller as well. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_02 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_02 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
-#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_02";
-int testno;
-int TST_TOTAL = 1;
-
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
-
- tst_rmdir();
-
-}
-
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
-{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
-}
-
-int main(int argc, char *argv[])
-{
- int fds[2], fl, i;
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
- }
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fds, 0) == -1) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (fl & O_NONBLOCK) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set non-blocking mode for fds[%d]",
- i);
- }
- close(fds[i]);
- }
-
- if (ltp_syscall(__NR_pipe2, fds, O_NONBLOCK) == -1) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_NONBLOCK) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((fl & O_NONBLOCK) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_NONBLOCK) does not set non-blocking mode for fds[%d]\n",
- i);
- }
- close(fds[i]);
- }
- tst_resm(TPASS, "pipe2(O_NONBLOCK) PASSED");
- cleanup();
- }
- }
- tst_exit();
-}
--
2.23.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 0/3] increase pipe2 coverage
2020-04-15 5:11 ` [LTP] [PATCH v2] " Yang Xu
@ 2020-04-16 7:28 ` Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API Yang Xu
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-16 7:28 UTC (permalink / raw)
To: ltp
Fixes:#625
v2->v3:
Add pipe2_02.c and pipe2_03.c test.
Noticed that:
Cases use SAFE_PIPE2 macro(it is in review[1]).
[1]https://patchwork.ozlabs.org/project/ltp/patch/20200409211255.1357-2-laniel_francis@privacyrequired.com/
Also, laniel has writed test case for pipe2 O_NONBLOCK flag, so I only
cover O_CLOEXEC and O_DIRECT flag.
Yang Xu (3):
syscalls/pipe2_01: convert into new API
syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag
syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
include/lapi/fcntl.h | 4 +
runtest/syscalls | 1 +
testcases/kernel/syscalls/pipe2/.gitignore | 2 +
testcases/kernel/syscalls/pipe2/pipe2_01.c | 234 +++++-------------
testcases/kernel/syscalls/pipe2/pipe2_02.c | 214 ++++------------
.../kernel/syscalls/pipe2/pipe2_02_child.c | 26 ++
testcases/kernel/syscalls/pipe2/pipe2_03.c | 157 ++++++++++++
7 files changed, 305 insertions(+), 333 deletions(-)
create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02_child.c
create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
--
2.23.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API
2020-04-16 7:28 ` [LTP] [PATCH v3 0/3] increase pipe2 coverage Yang Xu
@ 2020-04-16 7:29 ` Yang Xu
2020-04-21 8:19 ` Li Wang
2020-04-16 7:29 ` [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag Yang Xu
2 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-16 7:29 UTC (permalink / raw)
To: ltp
Also, add O_DIRECT flag test. This flag was introduced since
commit 9883035ae7ed ("pipes: add a "packetized pipe" mode for writing").
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
include/lapi/fcntl.h | 4 +
runtest/syscalls | 1 -
testcases/kernel/syscalls/pipe2/.gitignore | 1 -
testcases/kernel/syscalls/pipe2/pipe2_01.c | 234 ++++++---------------
testcases/kernel/syscalls/pipe2/pipe2_02.c | 175 ---------------
5 files changed, 65 insertions(+), 350 deletions(-)
delete mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index 24aa46e1f..576a18daf 100644
--- a/include/lapi/fcntl.h
+++ b/include/lapi/fcntl.h
@@ -9,6 +9,10 @@
#include <fcntl.h>
#include <sys/socket.h>
+#ifndef O_DIRECT
+# define O_DIRECT 040000
+#endif
+
#ifndef O_CLOEXEC
# define O_CLOEXEC 02000000
#endif
diff --git a/runtest/syscalls b/runtest/syscalls
index 44254d7da..79b671d50 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -911,7 +911,6 @@ pipe12 pipe12
pipe13 pipe13
pipe2_01 pipe2_01
-pipe2_02 pipe2_02
pivot_root01 pivot_root01
diff --git a/testcases/kernel/syscalls/pipe2/.gitignore b/testcases/kernel/syscalls/pipe2/.gitignore
index cd38bb309..42350bbdc 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1,2 +1 @@
/pipe2_01
-/pipe2_02
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_01.c b/testcases/kernel/syscalls/pipe2/pipe2_01.c
index 5f3ad8f0f..86f3f18d6 100644
--- a/testcases/kernel/syscalls/pipe2/pipe2_01.c
+++ b/testcases/kernel/syscalls/pipe2/pipe2_01.c
@@ -1,186 +1,74 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_01.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich?s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ed8cae8ba01348bfd83333f4648dd807b04d7f08 */
-/* says: */
-/* This patch introduces the new syscall pipe2 which is like pipe but it also */
-/* takes an additional parameter which takes a flag value. This patch */
-/* implements the handling of O_CLOEXEC for the flag. I did not add support */
-/* for the new syscall for the architectures which have a special sys_pipe */
-/* implementation. I think the maintainers of those archs have the chance to */
-/* go with the unified implementation but that's up to them. */
-/* */
-/* The implementation introduces do_pipe_flags. I did that instead of */
-/* changing all callers of do_pipe because some of the callers are written in */
-/* assembler. I would probably screw up changing the assembly code. To avoid */
-/* breaking code do_pipe is now a small wrapper around do_pipe_flags. Once */
-/* all callers are changed over to do_pipe_flags the old do_pipe function can */
-/* be removed. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_01 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_01 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
+// SPDX-License-Identifier: GPL-2.0-or-late
+/*
+ * Copyright (c) Ulrich Drepper <drepper@redhat.com>
+ * Copyright (c) International Business Machines Corp., 2009
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ *
+ * Author: Ulrich Drepper <drepper@redhat.com>
+ *
+ * History:
+ * Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com>
+ * Ported to LTP - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com>
+ * Converted into new api - Apri 15 2020 - Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ */
+#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_01";
-int testno;
-int TST_TOTAL = 1;
+#include "tst_test.h"
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
+static int fds[2];
- tst_rmdir();
-
-}
+static struct tcase {
+ int flags;
+ int check_cmd;
+ int check_read_side;
+ char *message;
+} tcases[] = {
+ {0, F_GETFD, 1, "Test pipe2 with 0 flag"},
+ {O_CLOEXEC, F_GETFD, 1, "Test pipe2 using O_CLOEXEC flag"},
+ /*
+ * It may get EINVAL error on older kernel because this flag was
+ * introduced since kernel 3.4. We only test flag in write side
+ * because this flag was used to make pipe buffer marked with the
+ * PIPE_BUF_FLAG_PACKET flag. In read side, kernel also check buffer
+ * flag instead of O_DIRECT. So it make no sense to check this flag
+ * in fds[0].
+ */
+ {O_DIRECT, F_GETFL, 0, "Test pipe2 using O_DIRECT flag"},
+ {O_NONBLOCK, F_GETFL, 1, "Test pipe2 using O_NONBLOCK flag"},
+};
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
+static void cleanup(void)
{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
+ if (fds[0] > 0)
+ SAFE_CLOSE(fds[0]);
+ if (fds[1] > 1)
+ SAFE_CLOSE(fds[1]);
}
-int main(int argc, char *argv[])
+static void verify_pipe2(unsigned int n)
{
- int fd[2], i, coe;
- int lc;
+ struct tcase *tc = &tcases[n];
+ int get_flag = 0, i = 0;
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
+ tst_res(TINFO, "%s ", tc->message);
+ SAFE_PIPE2(fds, tc->flags);
+ for (i = 0; i < 2; i++) {
+ if (i == 0 && !tc->check_read_side)
+ continue;
+ get_flag = SAFE_FCNTL(fds[i], tc->check_cmd);
+ if ((get_flag && tc->flags) || (tc->flags == get_flag))
+ tst_res(TPASS, "pipe2 fds[%d] gets expected flag(%d)", i, tc->flags);
+ else
+ tst_res(TFAIL, "pipe2 fds[%d] doesn't get expected flag(%d), get flag(%d)",
+ i, tc->flags, get_flag);
}
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fd, 0) != 0) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (coe & FD_CLOEXEC) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
-
- if (ltp_syscall(__NR_pipe2, fd, O_CLOEXEC) != 0) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_CLOEXEC) failed");
- }
- for (i = 0; i < 2; ++i) {
- coe = fcntl(fd[i], F_GETFD);
- if (coe == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((coe & FD_CLOEXEC) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_CLOEXEC) does not set close-on-exit for fd[%d]",
- i);
- }
- }
- close(fd[0]);
- close(fd[1]);
- tst_resm(TPASS, "pipe2(O_CLOEXEC) PASSED");
- cleanup();
- }
- }
- tst_exit();
+ cleanup();
}
+
+static struct tst_test test = {
+ .tcnt = ARRAY_SIZE(tcases),
+ .test = verify_pipe2,
+ .cleanup = cleanup,
+};
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c b/testcases/kernel/syscalls/pipe2/pipe2_02.c
deleted file mode 100644
index 39f02ff8c..000000000
--- a/testcases/kernel/syscalls/pipe2/pipe2_02.c
+++ /dev/null
@@ -1,175 +0,0 @@
-/******************************************************************************/
-/* */
-/* Copyright (c) Ulrich Drepper <drepper@redhat.com> */
-/* Copyright (c) International Business Machines Corp., 2009 */
-/* */
-/* This program is free software; you can redistribute it and/or modify */
-/* it under the terms of the GNU General Public License as published by */
-/* the Free Software Foundation; either version 2 of the License, or */
-/* (at your option) any later version. */
-/* */
-/* This program is distributed in the hope that it will be useful, */
-/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
-/* the GNU General Public License for more details. */
-/* */
-/* You should have received a copy of the GNU General Public License */
-/* along with this program; if not, write to the Free Software */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
-/* */
-/******************************************************************************/
-/******************************************************************************/
-/* */
-/* File: pipe2_02.c */
-/* */
-/* Description: This Program tests the new system call introduced in 2.6.27. */
-/* Ulrich?s comment as in: */
-/* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=be61a86d7237dd80510615f38ae21d6e1e98660c */
-/* which says: */
-/* This patch adds O_NONBLOCK support to pipe2. It is minimally more involved*/
-/* than the patches for eventfd et.al but still trivial. The interfaces of */
-/* the create_write_pipe and create_read_pipe helper functions were changed */
-/* and the one other caller as well. */
-/* The following test must be adjusted for architectures other than x86 and */
-/* x86-64 and in case the syscall numbers changed. */
-/* */
-/* Usage: <for command-line> */
-/* pipe2_02 [-c n] [-e][-i n] [-I x] [-p x] [-t] */
-/* where, -c n : Run n copies concurrently. */
-/* -e : Turn on errno logging. */
-/* -i n : Execute test n times. */
-/* -I x : Execute test for x seconds. */
-/* -P x : Pause for x seconds between iterations. */
-/* -t : Turn on syscall timing. */
-/* */
-/* Total Tests: 1 */
-/* */
-/* Test Name: pipe2_02 */
-/* */
-/* Author: Ulrich Drepper <drepper@redhat.com> */
-/* */
-/* History: Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com> */
-/* Ported to LTP */
-/* - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com> */
-/******************************************************************************/
-#include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/syscall.h>
-#include <errno.h>
-
-#include "test.h"
-#include "lapi/fcntl.h"
-#include "lapi/syscalls.h"
-
-char *TCID = "pipe2_02";
-int testno;
-int TST_TOTAL = 1;
-
-/* Extern Global Functions */
-/******************************************************************************/
-/* */
-/* Function: cleanup */
-/* */
-/* Description: Performs all one time clean up for this test on successful */
-/* completion, premature exit or failure. Closes all temporary */
-/* files, removes all temporary directories exits the test with */
-/* appropriate return code by calling tst_exit() function. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits calling tst_exit(). Non '0' return code. */
-/* On success - Exits calling tst_exit(). With '0' return code. */
-/* */
-/******************************************************************************/
-void cleanup(void)
-{
-
- tst_rmdir();
-
-}
-
-/* Local Functions */
-/******************************************************************************/
-/* */
-/* Function: setup */
-/* */
-/* Description: Performs all one time setup for this test. This function is */
-/* typically used to capture signals, create temporary dirs */
-/* and temporary files that may be used in the course of this */
-/* test. */
-/* */
-/* Input: None. */
-/* */
-/* Output: None. */
-/* */
-/* Return: On failure - Exits by calling cleanup(). */
-/* On success - returns 0. */
-/* */
-/******************************************************************************/
-void setup(void)
-{
- /* Capture signals if any */
- /* Create temporary directories */
- TEST_PAUSE;
- tst_tmpdir();
-}
-
-int main(int argc, char *argv[])
-{
- int fds[2], fl, i;
- int lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
- if ((tst_kvercmp(2, 6, 27)) < 0) {
- tst_brkm(TCONF,
- NULL,
- "This test can only run on kernels that are 2.6.27 and higher");
- }
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); ++lc) {
- tst_count = 0;
- for (testno = 0; testno < TST_TOTAL; ++testno) {
- if (ltp_syscall(__NR_pipe2, fds, 0) == -1) {
- tst_brkm(TFAIL, cleanup, "pipe2(0) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if (fl & O_NONBLOCK) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(0) set non-blocking mode for fds[%d]",
- i);
- }
- close(fds[i]);
- }
-
- if (ltp_syscall(__NR_pipe2, fds, O_NONBLOCK) == -1) {
- tst_brkm(TFAIL, cleanup,
- "pipe2(O_NONBLOCK) failed");
- }
- for (i = 0; i < 2; ++i) {
- fl = fcntl(fds[i], F_GETFL);
- if (fl == -1) {
- tst_brkm(TBROK, cleanup,
- "fcntl failed");
- }
- if ((fl & O_NONBLOCK) == 0) {
- tst_brkm(TFAIL,
- cleanup, "pipe2(O_NONBLOCK) does not set non-blocking mode for fds[%d]\n",
- i);
- }
- close(fds[i]);
- }
- tst_resm(TPASS, "pipe2(O_NONBLOCK) PASSED");
- cleanup();
- }
- }
- tst_exit();
-}
--
2.23.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag
2020-04-16 7:28 ` [LTP] [PATCH v3 0/3] increase pipe2 coverage Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API Yang Xu
@ 2020-04-16 7:29 ` Yang Xu
2020-04-21 8:49 ` Li Wang
2020-04-16 7:29 ` [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag Yang Xu
2 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-16 7:29 UTC (permalink / raw)
To: ltp
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/pipe2/.gitignore | 2 +
testcases/kernel/syscalls/pipe2/pipe2_02.c | 69 +++++++++++++++++++
.../kernel/syscalls/pipe2/pipe2_02_child.c | 26 +++++++
4 files changed, 98 insertions(+)
create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02_child.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 79b671d50..44254d7da 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -911,6 +911,7 @@ pipe12 pipe12
pipe13 pipe13
pipe2_01 pipe2_01
+pipe2_02 pipe2_02
pivot_root01 pivot_root01
diff --git a/testcases/kernel/syscalls/pipe2/.gitignore b/testcases/kernel/syscalls/pipe2/.gitignore
index 42350bbdc..786222de2 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1 +1,3 @@
/pipe2_01
+/pipe2_02
+/pipe2_02_child
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c b/testcases/kernel/syscalls/pipe2/pipe2_02.c
new file mode 100644
index 000000000..743d78c58
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_02.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ * This case is designed to test the basic functionality about the
+ * O_CLOEXEC flag of pipe2.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include "lapi/fcntl.h"
+#include "tst_test.h"
+
+#define TESTBIN "pipe2_02_child"
+static int fds[2];
+
+static void cleanup(void)
+{
+ if (fds[0] > 0)
+ SAFE_CLOSE(fds[0]);
+ if (fds[1] > 0)
+ SAFE_CLOSE(fds[1]);
+}
+
+static void verify_pipe2(void)
+{
+ int pid, status;
+ char buf[20];
+
+ SAFE_PIPE2(fds, O_CLOEXEC);
+ sprintf(buf, "%d", fds[1]);
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ if (execlp(TESTBIN, TESTBIN, buf, NULL))
+ exit(2);
+ }
+
+ SAFE_WAIT(&status);
+ if (WIFEXITED(status)) {
+ switch (WEXITSTATUS(status)) {
+ case 0:
+ tst_res(TPASS, "test O_CLOEXEC for pipe2 success");
+ break;
+ case 1:
+ tst_res(TFAIL, "test O_CLOEXEC for pipe2 failed");
+ break;
+ default:
+ tst_brk(TBROK, "execlp() failed");
+ }
+ } else {
+ tst_brk(TBROK, "%s exits with unexpected error", TESTBIN);
+ }
+ cleanup();
+}
+
+static const char *const resfile[] = {
+ TESTBIN,
+ NULL,
+};
+
+static struct tst_test test = {
+ .resource_files = resfile,
+ .cleanup = cleanup,
+ .forks_child = 1,
+ .needs_root = 1,
+ .test_all = verify_pipe2,
+};
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02_child.c b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
new file mode 100644
index 000000000..d5ed68cf7
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+
+int main(int argc, char **argv)
+{
+ int ret;
+ int fd;
+
+ if (argc != 2) {
+ fprintf(stderr, "Only two arguments: %s <fd>\n", argv[0]);
+ exit(1);
+ }
+
+ fd = atoi(argv[1]);
+ ret = write(fd, "x", 1);
+
+ return ret != -1;
+}
--
2.23.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-16 7:28 ` [LTP] [PATCH v3 0/3] increase pipe2 coverage Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag Yang Xu
@ 2020-04-16 7:29 ` Yang Xu
2020-04-17 9:44 ` Li Wang
2020-04-21 9:53 ` Li Wang
2 siblings, 2 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-16 7:29 UTC (permalink / raw)
To: ltp
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
runtest/syscalls | 1 +
testcases/kernel/syscalls/pipe2/.gitignore | 1 +
testcases/kernel/syscalls/pipe2/pipe2_03.c | 157 +++++++++++++++++++++
3 files changed, 159 insertions(+)
create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 44254d7da..4d8ebc5a3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -912,6 +912,7 @@ pipe13 pipe13
pipe2_01 pipe2_01
pipe2_02 pipe2_02
+pipe2_03 pipe2_03
pivot_root01 pivot_root01
diff --git a/testcases/kernel/syscalls/pipe2/.gitignore b/testcases/kernel/syscalls/pipe2/.gitignore
index 786222de2..4cc5acaf1 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1,3 +1,4 @@
/pipe2_01
/pipe2_02
/pipe2_02_child
+/pipe2_03
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_03.c b/testcases/kernel/syscalls/pipe2/pipe2_03.c
new file mode 100644
index 000000000..0b5d37dd0
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ * This case is designed to test the basic functionality about the
+ * O_DIRECT flag of pipe2.
+ *
+ * It includes three sub tests.
+ * 1) Each write(2) to the pipe is dealt with as a separate packet, and
+ * read(2)s from the pipe will read one packet at a time.
+ * 2) Writes of greater than PIPE_BUF bytes (see pipe(7)) will be split
+ * into multiple packet.
+ * 3)If a read(2) specifies a buffer size that is smaller than the next
+ * packet, then the requested number of bytes are read, and the excess
+ * bytes in the packet are discarded.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <linux/limits.h>
+#include "lapi/fcntl.h"
+#include "tst_test.h"
+
+static int fds[2], packet_num, pipe_size;
+static char *wrbuf;
+static char *rdbuf;
+static void check_peer_rw(void);
+static void check_split(void);
+static void check_discard(void);
+
+static void (*test_func[])(void) = {check_peer_rw, check_split, check_discard};
+
+static void check_peer_rw(void)
+{
+ int i, pid;
+
+ SAFE_PIPE2(fds, O_DIRECT | O_NONBLOCK);
+ for (i = 0; i < packet_num; i++)
+ SAFE_WRITE(1, fds[1], "x", 1);
+
+ TEST(write(fds[1], "x", 1));
+ if (TST_RET != -1) {
+ tst_res(TFAIL, "write succeeded unexpectedly");
+ } else {
+ if (TST_ERR == EAGAIN)
+ tst_res(TPASS, "Each write(2) uses a separate packet");
+ else
+ tst_res(TFAIL | TTERRNO, "write failed, expected EAGAIN but got");
+ }
+ pid = SAFE_FORK();
+ if (!pid) {
+ memset(rdbuf, 0, pipe_size);
+ for (i = 0; i < packet_num; i++) {
+ TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
+ if (TST_RET != 1)
+ tst_res(TFAIL,
+ "Each read(2) doesn't read a separate packet, return %ld", TST_RET);
+ }
+ tst_res(TPASS, "Each read(2) reads a separate packet");
+ }
+}
+
+static void check_split(void)
+{
+ int i, pid;
+
+ SAFE_PIPE2(fds, O_DIRECT);
+ SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF * 2);
+
+ pid = SAFE_FORK();
+ if (!pid) {
+ memset(rdbuf, 0, pipe_size);
+ for (i = 0; i < 2; i++) {
+ TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
+ if (TST_RET != PIPE_BUF)
+ tst_res(TFAIL,
+ "write(higner than PIPE_BUF) split into multiple packet, return %ld", TST_RET);
+ }
+ tst_res(TPASS, "write(higner than PIPE_BUF) split into multiple packet");
+ }
+}
+
+static void check_discard(void)
+{
+ int pid;
+ char tmp_buf[20];
+ char tmp_secondbuf[20];
+
+ SAFE_PIPE2(fds, O_DIRECT);
+ SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF);
+ SAFE_WRITE(1, fds[1], "1", 1);
+
+ pid = SAFE_FORK();
+ if (!pid) {
+ TEST(SAFE_READ(0, fds[0], tmp_buf, 20));
+ if (TST_RET != 20)
+ tst_res(TFAIL,
+ "the excess bytes in the packet isn't discarded by read, return %ld", TST_RET);
+ TEST(SAFE_READ(0, fds[0], tmp_secondbuf, 20));
+ if (TST_RET == 1) {
+ if (!strcmp(tmp_secondbuf, "1"))
+ tst_res(TPASS,
+ "the excess bytes in the packet is discarded by read, only read 1");
+ else
+ tst_res(TFAIL,
+ "the excess bytes in the packet is discarded by read, expect 1 got %s", tmp_secondbuf);
+ }
+ }
+}
+
+static void verify_pipe2(unsigned int n)
+{
+ int pid;
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ (*test_func[n])();
+ tst_reap_children();
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
+ }
+ tst_reap_children();
+}
+
+static void setup(void)
+{
+ SAFE_PIPE2(fds, O_DIRECT);
+ pipe_size = SAFE_FCNTL(fds[1], F_GETPIPE_SZ);
+ wrbuf = SAFE_MALLOC(PIPE_BUF * 2);
+ rdbuf = SAFE_MALLOC(pipe_size);
+ memset(wrbuf, 'x', PIPE_BUF * 2);
+ packet_num = pipe_size / PIPE_BUF;
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
+}
+
+static void cleanup(void)
+{
+ if (fds[0] > 0)
+ SAFE_CLOSE(fds[0]);
+ if (fds[1] > 0)
+ SAFE_CLOSE(fds[1]);
+ if (wrbuf)
+ free(wrbuf);
+ if (rdbuf)
+ free(rdbuf);
+}
+
+static struct tst_test test = {
+ .setup = setup,
+ .cleanup = cleanup,
+ .forks_child = 1,
+ .test = verify_pipe2,
+ .tcnt = ARRAY_SIZE(test_func),
+};
--
2.23.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-16 7:29 ` [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag Yang Xu
@ 2020-04-17 9:44 ` Li Wang
2020-04-17 9:56 ` Yang Xu
2020-04-21 9:53 ` Li Wang
1 sibling, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-17 9:44 UTC (permalink / raw)
To: ltp
Hi Xu,
I feel confused that is this test duplicated with the one Lanie's patch2/2?
why using the same test name?
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200417/e7dba96b/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-17 9:44 ` Li Wang
@ 2020-04-17 9:56 ` Yang Xu
2020-04-17 10:13 ` Li Wang
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-17 9:56 UTC (permalink / raw)
To: ltp
Hi Li
> Hi Xu,
>
> I feel confused that is this test duplicated with the one Lanie's
> patch2/2? why using the same test name?
My pipe2_03.c is to test O_DIRECT flag, Lanie's patch is to test
O_NONBLOCK flag as my perivous email said, see[1].
I don't think it is a problem(I need to generate patch-set, so named
pipe2_03.c is better).
[1]http://lists.linux.it/pipermail/ltp/2020-April/016611.html
Best Regards
Yang Xu
>
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-17 9:56 ` Yang Xu
@ 2020-04-17 10:13 ` Li Wang
2020-04-17 10:50 ` Li Wang
0 siblings, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-17 10:13 UTC (permalink / raw)
To: ltp
On Fri, Apr 17, 2020 at 5:56 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> Hi Li
>
> > Hi Xu,
> >
> > I feel confused that is this test duplicated with the one Lanie's
> > patch2/2? why using the same test name?
> My pipe2_03.c is to test O_DIRECT flag, Lanie's patch is to test
> O_NONBLOCK flag as my perivous email said, see[1].
>
> I don't think it is a problem(I need to generate patch-set, so named
> pipe2_03.c is better).
>
But that will cause git patch confiliction becasue same file name, isn't it?
I'm going to apply Lanie's two patches(modified on the git.v3), and once it
done, your patch will broken on git confiliction.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200417/9be42f4e/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-17 10:13 ` Li Wang
@ 2020-04-17 10:50 ` Li Wang
0 siblings, 0 replies; 25+ messages in thread
From: Li Wang @ 2020-04-17 10:50 UTC (permalink / raw)
To: ltp
On Fri, Apr 17, 2020 at 6:13 PM Li Wang <liwang@redhat.com> wrote:
>
>
> On Fri, Apr 17, 2020 at 5:56 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> wrote:
>
>> Hi Li
>>
>> > Hi Xu,
>> >
>> > I feel confused that is this test duplicated with the one Lanie's
>> > patch2/2? why using the same test name?
>> My pipe2_03.c is to test O_DIRECT flag, Lanie's patch is to test
>> O_NONBLOCK flag as my perivous email said, see[1].
>>
>> I don't think it is a problem(I need to generate patch-set, so named
>> pipe2_03.c is better).
>>
>
> But that will cause git patch confiliction becasue same file name, isn't
> it?
> I'm going to apply Lanie's two patches(modified on the git.v3), and once
> it done, your patch will broken on git confiliction.
>
>
One feasible way is that I help to rename Lanie's pipe2_03.c to pipe2_04.c
and apply the patches first, then you do rebase your codes on the
latest commit. Is that OK for you?
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200417/919bae77/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API
2020-04-16 7:29 ` [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API Yang Xu
@ 2020-04-21 8:19 ` Li Wang
2020-04-21 8:27 ` Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-21 8:19 UTC (permalink / raw)
To: ltp
Hi Xu,
On Thu, Apr 16, 2020 at 3:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> Also, add O_DIRECT flag test. This flag was introduced since
> commit 9883035ae7ed ("pipes: add a "packetized pipe" mode for writing").
>
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
> include/lapi/fcntl.h | 4 +
> runtest/syscalls | 1 -
> testcases/kernel/syscalls/pipe2/.gitignore | 1 -
> testcases/kernel/syscalls/pipe2/pipe2_01.c | 234 ++++++---------------
> testcases/kernel/syscalls/pipe2/pipe2_02.c | 175 ---------------
> 5 files changed, 65 insertions(+), 350 deletions(-)
> delete mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
>
> diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
> index 24aa46e1f..576a18daf 100644
> --- a/include/lapi/fcntl.h
> +++ b/include/lapi/fcntl.h
> @@ -9,6 +9,10 @@
> #include <fcntl.h>
> #include <sys/socket.h>
>
> +#ifndef O_DIRECT
> +# define O_DIRECT 040000
> +#endif
> +
> #ifndef O_CLOEXEC
> # define O_CLOEXEC 02000000
> #endif
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 44254d7da..79b671d50 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -911,7 +911,6 @@ pipe12 pipe12
> pipe13 pipe13
>
> pipe2_01 pipe2_01
> -pipe2_02 pipe2_02
>
> pivot_root01 pivot_root01
>
> diff --git a/testcases/kernel/syscalls/pipe2/.gitignore
> b/testcases/kernel/syscalls/pipe2/.gitignore
> index cd38bb309..42350bbdc 100644
> --- a/testcases/kernel/syscalls/pipe2/.gitignore
> +++ b/testcases/kernel/syscalls/pipe2/.gitignore
> @@ -1,2 +1 @@
> /pipe2_01
> -/pipe2_02
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_01.c
> b/testcases/kernel/syscalls/pipe2/pipe2_01.c
> index 5f3ad8f0f..86f3f18d6 100644
> --- a/testcases/kernel/syscalls/pipe2/pipe2_01.c
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_01.c
> @@ -1,186 +1,74 @@
>
> -/******************************************************************************/
> -/*
> */
> -/* Copyright (c) Ulrich Drepper <drepper@redhat.com>
> */
> -/* Copyright (c) International Business Machines Corp., 2009
> */
> -/*
> */
> -/* This program is free software; you can redistribute it and/or modify
> */
> -/* it under the terms of the GNU General Public License as published by
> */
> -/* the Free Software Foundation; either version 2 of the License, or
> */
> -/* (at your option) any later version.
> */
> -/*
> */
> -/* This program is distributed in the hope that it will be useful,
> */
> -/* but WITHOUT ANY WARRANTY; without even the implied warranty of
> */
> -/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> */
> -/* the GNU General Public License for more details.
> */
> -/*
> */
> -/* You should have received a copy of the GNU General Public License
> */
> -/* along with this program; if not, write to the Free Software
> */
> -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA */
> -/*
> */
>
> -/******************************************************************************/
>
> -/******************************************************************************/
> -/*
> */
> -/* File: pipe2_01.c
> */
> -/*
> */
> -/* Description: This Program tests the new system call introduced in
> 2.6.27. */
> -/* Ulrich?s comment as in:
> */
> -/*
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ed8cae8ba01348bfd83333f4648dd807b04d7f08
> */
> -/* says:
> */
> -/* This patch introduces the new syscall pipe2 which is like pipe but it
> also */
> -/* takes an additional parameter which takes a flag value. This patch
> */
> -/* implements the handling of O_CLOEXEC for the flag. I did not add
> support */
> -/* for the new syscall for the architectures which have a special
> sys_pipe */
> -/* implementation. I think the maintainers of those archs have the
> chance to */
> -/* go with the unified implementation but that's up to them.
> */
> -/*
> */
> -/* The implementation introduces do_pipe_flags. I did that instead of
> */
> -/* changing all callers of do_pipe because some of the callers are
> written in */
> -/* assembler. I would probably screw up changing the assembly code. To
> avoid */
> -/* breaking code do_pipe is now a small wrapper around do_pipe_flags.
> Once */
> -/* all callers are changed over to do_pipe_flags the old do_pipe function
> can */
> -/* be removed.
> */
> -/* The following test must be adjusted for architectures other than x86
> and */
> -/* x86-64 and in case the syscall numbers changed.
> */
> -/*
> */
> -/* Usage: <for command-line>
> */
> -/* pipe2_01 [-c n] [-e][-i n] [-I x] [-p x] [-t]
> */
> -/* where, -c n : Run n copies concurrently.
> */
> -/* -e : Turn on errno logging.
> */
> -/* -i n : Execute test n times.
> */
> -/* -I x : Execute test for x seconds.
> */
> -/* -P x : Pause for x seconds between iterations.
> */
> -/* -t : Turn on syscall timing.
> */
> -/*
> */
> -/* Total Tests: 1
> */
> -/*
> */
> -/* Test Name: pipe2_01
> */
> -/*
> */
> -/* Author: Ulrich Drepper <drepper@redhat.com>
> */
> -/*
> */
> -/* History: Created - Jan 13 2009 - Ulrich Drepper <
> drepper@redhat.com> */
> -/* Ported to LTP
> */
> -/* - Jan 13 2009 - Subrata <
> subrata@linux.vnet.ibm.com> */
>
> -/******************************************************************************/
> -#include <fcntl.h>
> +// SPDX-License-Identifier: GPL-2.0-or-late
> +/*
> + * Copyright (c) Ulrich Drepper <drepper@redhat.com>
> + * Copyright (c) International Business Machines Corp., 2009
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + *
> + * Author: Ulrich Drepper <drepper@redhat.com>
> + *
> + * History:
> + * Created - Jan 13 2009 - Ulrich Drepper <drepper@redhat.com>
> + * Ported to LTP - Jan 13 2009 - Subrata <subrata@linux.vnet.ibm.com>
> + * Converted into new api - Apri 15 2020 - Yang Xu <
> xuyang2018.jy@cn.fujitsu.com>
> + */
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <unistd.h>
> -#include <sys/syscall.h>
> -#include <errno.h>
> -
> -#include "test.h"
> #include "lapi/fcntl.h"
> -#include "lapi/syscalls.h"
> -
> -char *TCID = "pipe2_01";
> -int testno;
> -int TST_TOTAL = 1;
> +#include "tst_test.h"
>
> -/* Extern Global Functions */
>
> -/******************************************************************************/
> -/*
> */
> -/* Function: cleanup
> */
> -/*
> */
> -/* Description: Performs all one time clean up for this test on
> successful */
> -/* completion, premature exit or failure. Closes all
> temporary */
> -/* files, removes all temporary directories exits the test
> with */
> -/* appropriate return code by calling tst_exit() function.
> */
> -/*
> */
> -/* Input: None.
> */
> -/*
> */
> -/* Output: None.
> */
> -/*
> */
> -/* Return: On failure - Exits calling tst_exit(). Non '0' return
> code. */
> -/* On success - Exits calling tst_exit(). With '0' return
> code. */
> -/*
> */
>
> -/******************************************************************************/
> -void cleanup(void)
> -{
> +static int fds[2];
>
> - tst_rmdir();
> -
> -}
> +static struct tcase {
> + int flags;
> + int check_cmd;
>
The 'check_cmd' sounds strange here since we don't do any check for the
cmd, can we use 'cmd' directly?
> + int check_read_side;
>
A pipe has a read-end and a write-end. So I prefer to call this
'check_read_end', do you agree?
> + char *message;
> +} tcases[] = {
> + {0, F_GETFD, 1, "Test pipe2 with 0 flag"},
> + {O_CLOEXEC, F_GETFD, 1, "Test pipe2 using O_CLOEXEC flag"},
> + /*
> + * It may get EINVAL error on older kernel because this flag was
> + * introduced since kernel 3.4. We only test flag in write side
> + * because this flag was used to make pipe buffer marked with the
> + * PIPE_BUF_FLAG_PACKET flag. In read side, kernel also check
> buffer
> + * flag instead of O_DIRECT. So it make no sense to check this flag
> + * in fds[0].
> + */
> + {O_DIRECT, F_GETFL, 0, "Test pipe2 using O_DIRECT flag"},
> + {O_NONBLOCK, F_GETFL, 1, "Test pipe2 using O_NONBLOCK flag"},
> +};
>
> -/* Local Functions */
>
> -/******************************************************************************/
> -/*
> */
> -/* Function: setup
> */
> -/*
> */
> -/* Description: Performs all one time setup for this test. This function
> is */
> -/* typically used to capture signals, create temporary dirs
> */
> -/* and temporary files that may be used in the course of
> this */
> -/* test.
> */
> -/*
> */
> -/* Input: None.
> */
> -/*
> */
> -/* Output: None.
> */
> -/*
> */
> -/* Return: On failure - Exits by calling cleanup().
> */
> -/* On success - returns 0.
> */
> -/*
> */
>
> -/******************************************************************************/
> -void setup(void)
> +static void cleanup(void)
> {
> - /* Capture signals if any */
> - /* Create temporary directories */
> - TEST_PAUSE;
> - tst_tmpdir();
> + if (fds[0] > 0)
> + SAFE_CLOSE(fds[0]);
> + if (fds[1] > 1)
> + SAFE_CLOSE(fds[1]);
> }
>
> -int main(int argc, char *argv[])
> +static void verify_pipe2(unsigned int n)
> {
> - int fd[2], i, coe;
> - int lc;
> + struct tcase *tc = &tcases[n];
> + int get_flag = 0, i = 0;
>
> - tst_parse_opts(argc, argv, NULL, NULL);
> - if ((tst_kvercmp(2, 6, 27)) < 0) {
> - tst_brkm(TCONF,
> - NULL,
> - "This test can only run on kernels that are
> 2.6.27 and higher");
> + tst_res(TINFO, "%s ", tc->message);
> + SAFE_PIPE2(fds, tc->flags);
> + for (i = 0; i < 2; i++) {
> + if (i == 0 && !tc->check_read_side)
> + continue;
> + get_flag = SAFE_FCNTL(fds[i], tc->check_cmd);
> + if ((get_flag && tc->flags) || (tc->flags == get_flag))
> + tst_res(TPASS, "pipe2 fds[%d] gets expected
> flag(%d)", i, tc->flags);
> + else
> + tst_res(TFAIL, "pipe2 fds[%d] doesn't get expected
> flag(%d), get flag(%d)",
> + i, tc->flags, get_flag);
> }
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); ++lc) {
> - tst_count = 0;
> - for (testno = 0; testno < TST_TOTAL; ++testno) {
> - if (ltp_syscall(__NR_pipe2, fd, 0) != 0) {
> - tst_brkm(TFAIL, cleanup, "pipe2(0)
> failed");
> - }
> - for (i = 0; i < 2; ++i) {
> - coe = fcntl(fd[i], F_GETFD);
> - if (coe == -1) {
> - tst_brkm(TBROK, cleanup,
> - "fcntl failed");
> - }
> - if (coe & FD_CLOEXEC) {
> - tst_brkm(TFAIL,
> - cleanup, "pipe2(0) set
> close-on-exit for fd[%d]",
> - i);
> - }
> - }
> - close(fd[0]);
> - close(fd[1]);
> -
> - if (ltp_syscall(__NR_pipe2, fd, O_CLOEXEC) != 0) {
> - tst_brkm(TFAIL, cleanup,
> - "pipe2(O_CLOEXEC) failed");
> - }
> - for (i = 0; i < 2; ++i) {
> - coe = fcntl(fd[i], F_GETFD);
> - if (coe == -1) {
> - tst_brkm(TBROK, cleanup,
> - "fcntl failed");
> - }
> - if ((coe & FD_CLOEXEC) == 0) {
> - tst_brkm(TFAIL,
> - cleanup,
> "pipe2(O_CLOEXEC) does not set close-on-exit for fd[%d]",
> - i);
> - }
> - }
> - close(fd[0]);
> - close(fd[1]);
> - tst_resm(TPASS, "pipe2(O_CLOEXEC) PASSED");
> - cleanup();
> - }
> - }
> - tst_exit();
> + cleanup();
> }
> +
> +static struct tst_test test = {
> + .tcnt = ARRAY_SIZE(tcases),
> + .test = verify_pipe2,
> + .cleanup = cleanup,
> +};
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c
> b/testcases/kernel/syscalls/pipe2/pipe2_02.c
> deleted file mode 100644
> index 39f02ff8c..000000000
> --- a/testcases/kernel/syscalls/pipe2/pipe2_02.c
> +++ /dev/null
> @@ -1,175 +0,0 @@
>
> -/******************************************************************************/
> -/*
> */
> -/* Copyright (c) Ulrich Drepper <drepper@redhat.com>
> */
> -/* Copyright (c) International Business Machines Corp., 2009
> */
> -/*
> */
> -/* This program is free software; you can redistribute it and/or modify
> */
> -/* it under the terms of the GNU General Public License as published by
> */
> -/* the Free Software Foundation; either version 2 of the License, or
> */
> -/* (at your option) any later version.
> */
> -/*
> */
> -/* This program is distributed in the hope that it will be useful,
> */
> -/* but WITHOUT ANY WARRANTY; without even the implied warranty of
> */
> -/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> */
> -/* the GNU General Public License for more details.
> */
> -/*
> */
> -/* You should have received a copy of the GNU General Public License
> */
> -/* along with this program; if not, write to the Free Software
> */
> -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA */
> -/*
> */
>
> -/******************************************************************************/
>
> -/******************************************************************************/
> -/*
> */
> -/* File: pipe2_02.c
> */
> -/*
> */
> -/* Description: This Program tests the new system call introduced in
> 2.6.27. */
> -/* Ulrich?s comment as in:
> */
> -/*
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=be61a86d7237dd80510615f38ae21d6e1e98660c
> */
> -/* which says:
> */
> -/* This patch adds O_NONBLOCK support to pipe2. It is minimally more
> involved*/
> -/* than the patches for eventfd et.al but still trivial. The interfaces
> of */
> -/* the create_write_pipe and create_read_pipe helper functions were
> changed */
> -/* and the one other caller as well.
> */
> -/* The following test must be adjusted for architectures other than x86
> and */
> -/* x86-64 and in case the syscall numbers changed.
> */
> -/*
> */
> -/* Usage: <for command-line>
> */
> -/* pipe2_02 [-c n] [-e][-i n] [-I x] [-p x] [-t]
> */
> -/* where, -c n : Run n copies concurrently.
> */
> -/* -e : Turn on errno logging.
> */
> -/* -i n : Execute test n times.
> */
> -/* -I x : Execute test for x seconds.
> */
> -/* -P x : Pause for x seconds between iterations.
> */
> -/* -t : Turn on syscall timing.
> */
> -/*
> */
> -/* Total Tests: 1
> */
> -/*
> */
> -/* Test Name: pipe2_02
> */
> -/*
> */
> -/* Author: Ulrich Drepper <drepper@redhat.com>
> */
> -/*
> */
> -/* History: Created - Jan 13 2009 - Ulrich Drepper <
> drepper@redhat.com> */
> -/* Ported to LTP
> */
> -/* - Jan 13 2009 - Subrata <
> subrata@linux.vnet.ibm.com> */
>
> -/******************************************************************************/
> -#include <fcntl.h>
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <sys/syscall.h>
> -#include <errno.h>
> -
> -#include "test.h"
> -#include "lapi/fcntl.h"
> -#include "lapi/syscalls.h"
> -
> -char *TCID = "pipe2_02";
> -int testno;
> -int TST_TOTAL = 1;
> -
> -/* Extern Global Functions */
>
> -/******************************************************************************/
> -/*
> */
> -/* Function: cleanup
> */
> -/*
> */
> -/* Description: Performs all one time clean up for this test on
> successful */
> -/* completion, premature exit or failure. Closes all
> temporary */
> -/* files, removes all temporary directories exits the test
> with */
> -/* appropriate return code by calling tst_exit() function.
> */
> -/*
> */
> -/* Input: None.
> */
> -/*
> */
> -/* Output: None.
> */
> -/*
> */
> -/* Return: On failure - Exits calling tst_exit(). Non '0' return
> code. */
> -/* On success - Exits calling tst_exit(). With '0' return
> code. */
> -/*
> */
>
> -/******************************************************************************/
> -void cleanup(void)
> -{
> -
> - tst_rmdir();
> -
> -}
> -
> -/* Local Functions */
>
> -/******************************************************************************/
> -/*
> */
> -/* Function: setup
> */
> -/*
> */
> -/* Description: Performs all one time setup for this test. This function
> is */
> -/* typically used to capture signals, create temporary dirs
> */
> -/* and temporary files that may be used in the course of
> this */
> -/* test.
> */
> -/*
> */
> -/* Input: None.
> */
> -/*
> */
> -/* Output: None.
> */
> -/*
> */
> -/* Return: On failure - Exits by calling cleanup().
> */
> -/* On success - returns 0.
> */
> -/*
> */
>
> -/******************************************************************************/
> -void setup(void)
> -{
> - /* Capture signals if any */
> - /* Create temporary directories */
> - TEST_PAUSE;
> - tst_tmpdir();
> -}
> -
> -int main(int argc, char *argv[])
> -{
> - int fds[2], fl, i;
> - int lc;
> -
> - tst_parse_opts(argc, argv, NULL, NULL);
> - if ((tst_kvercmp(2, 6, 27)) < 0) {
> - tst_brkm(TCONF,
> - NULL,
> - "This test can only run on kernels that are
> 2.6.27 and higher");
> - }
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); ++lc) {
> - tst_count = 0;
> - for (testno = 0; testno < TST_TOTAL; ++testno) {
> - if (ltp_syscall(__NR_pipe2, fds, 0) == -1) {
> - tst_brkm(TFAIL, cleanup, "pipe2(0)
> failed");
> - }
> - for (i = 0; i < 2; ++i) {
> - fl = fcntl(fds[i], F_GETFL);
> - if (fl == -1) {
> - tst_brkm(TBROK, cleanup,
> - "fcntl failed");
> - }
> - if (fl & O_NONBLOCK) {
> - tst_brkm(TFAIL,
> - cleanup, "pipe2(0) set
> non-blocking mode for fds[%d]",
> - i);
> - }
> - close(fds[i]);
> - }
> -
> - if (ltp_syscall(__NR_pipe2, fds, O_NONBLOCK) ==
> -1) {
> - tst_brkm(TFAIL, cleanup,
> - "pipe2(O_NONBLOCK) failed");
> - }
> - for (i = 0; i < 2; ++i) {
> - fl = fcntl(fds[i], F_GETFL);
> - if (fl == -1) {
> - tst_brkm(TBROK, cleanup,
> - "fcntl failed");
> - }
> - if ((fl & O_NONBLOCK) == 0) {
> - tst_brkm(TFAIL,
> - cleanup,
> "pipe2(O_NONBLOCK) does not set non-blocking mode for fds[%d]\n",
> - i);
> - }
> - close(fds[i]);
> - }
> - tst_resm(TPASS, "pipe2(O_NONBLOCK) PASSED");
> - cleanup();
> - }
> - }
> - tst_exit();
> -}
> --
> 2.23.0
>
>
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200421/407bef04/attachment-0001.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API
2020-04-21 8:19 ` Li Wang
@ 2020-04-21 8:27 ` Yang Xu
0 siblings, 0 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-21 8:27 UTC (permalink / raw)
To: ltp
Hi Li
> -}
> +static struct tcase {
> +? ? ? ?int flags;
> +? ? ? ?int check_cmd;
>
>
> The 'check_cmd' sounds strange here since we don't do any check for the
> cmd, can we use 'cmd' directly?
Yes.
>
> +? ? ? ?int check_read_side;
>
>
> A pipe has a read-end and a write-end. So I prefer to call this
> 'check_read_end', do you agree?
Agree. man-pages also said "the read/write end of the pipe". Thanks for
pointing out this.
>
> +? ? ? ?char *message;
> +} tcases[] = {
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag
2020-04-16 7:29 ` [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag Yang Xu
@ 2020-04-21 8:49 ` Li Wang
2020-04-21 9:41 ` Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-21 8:49 UTC (permalink / raw)
To: ltp
On Thu, Apr 16, 2020 at 3:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
> runtest/syscalls | 1 +
> testcases/kernel/syscalls/pipe2/.gitignore | 2 +
> testcases/kernel/syscalls/pipe2/pipe2_02.c | 69 +++++++++++++++++++
> .../kernel/syscalls/pipe2/pipe2_02_child.c | 26 +++++++
> 4 files changed, 98 insertions(+)
> create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
> create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02_child.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 79b671d50..44254d7da 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -911,6 +911,7 @@ pipe12 pipe12
> pipe13 pipe13
>
> pipe2_01 pipe2_01
> +pipe2_02 pipe2_02
>
> pivot_root01 pivot_root01
>
> diff --git a/testcases/kernel/syscalls/pipe2/.gitignore
> b/testcases/kernel/syscalls/pipe2/.gitignore
> index 42350bbdc..786222de2 100644
> --- a/testcases/kernel/syscalls/pipe2/.gitignore
> +++ b/testcases/kernel/syscalls/pipe2/.gitignore
> @@ -1 +1,3 @@
> /pipe2_01
> +/pipe2_02
> +/pipe2_02_child
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c
> b/testcases/kernel/syscalls/pipe2/pipe2_02.c
> new file mode 100644
> index 000000000..743d78c58
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_02.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> + *
> + * This case is designed to test the basic functionality about the
> + * O_CLOEXEC flag of pipe2.
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include "lapi/fcntl.h"
> +#include "tst_test.h"
> +
> +#define TESTBIN "pipe2_02_child"
> +static int fds[2];
> +
> +static void cleanup(void)
> +{
> + if (fds[0] > 0)
> + SAFE_CLOSE(fds[0]);
> + if (fds[1] > 0)
> + SAFE_CLOSE(fds[1]);
> +}
> +
> +static void verify_pipe2(void)
> +{
> + int pid, status;
> + char buf[20];
> +
> + SAFE_PIPE2(fds, O_CLOEXEC);
> + sprintf(buf, "%d", fds[1]);
> + pid = SAFE_FORK();
> + if (pid == 0) {
> + if (execlp(TESTBIN, TESTBIN, buf, NULL))
> + exit(2);
>
Do we really need the if() condition and exit(2)? AFAIK, the exec() family
of functions replaces the current process image with a new process and
returns zero if succeeded.
> + }
> +
> + SAFE_WAIT(&status);
> + if (WIFEXITED(status)) {
> + switch (WEXITSTATUS(status)) {
> + case 0:
> + tst_res(TPASS, "test O_CLOEXEC for pipe2 success");
> + break;
> + case 1:
> + tst_res(TFAIL, "test O_CLOEXEC for pipe2 failed");
> + break;
> + default:
> + tst_brk(TBROK, "execlp() failed");
> + }
> + } else {
> + tst_brk(TBROK, "%s exits with unexpected error", TESTBIN);
> + }
> + cleanup();
> +}
> +
> +static const char *const resfile[] = {
> + TESTBIN,
> + NULL,
> +};
> +
> +static struct tst_test test = {
> + .resource_files = resfile,
> + .cleanup = cleanup,
> + .forks_child = 1,
> + .needs_root = 1,
> + .test_all = verify_pipe2,
> +};
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> new file mode 100644
> index 000000000..d5ed68cf7
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +
> +int main(int argc, char **argv)
> +{
> + int ret;
> + int fd;
> +
> + if (argc != 2) {
> + fprintf(stderr, "Only two arguments: %s <fd>\n", argv[0]);
> + exit(1);
> + }
> +
> + fd = atoi(argv[1]);
> + ret = write(fd, "x", 1);
> +
> + return ret != -1;
To check if the return value equals -1 maybe not a good idea to confirm the
'fd' is closed. That only proof it failed to write "x" to the fd, we are
not sure if that exists other errors.
What about using the fcntl() in the fd status check?
if (fcntl(fd, F_GETFL) < 0 && errno == EBADF)
return 0;
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200421/7ffc5ae2/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag
2020-04-21 8:49 ` Li Wang
@ 2020-04-21 9:41 ` Yang Xu
2020-04-21 9:59 ` Li Wang
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-21 9:41 UTC (permalink / raw)
To: ltp
Hi Li
>
>
> On Thu, Apr 16, 2020 at 3:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>>
> ---
> ?runtest/syscalls? ? ? ? ? ? ? ? ? ? ? ? ? ? ? |? 1 +
> ?testcases/kernel/syscalls/pipe2/.gitignore? ? |? 2 +
> ?testcases/kernel/syscalls/pipe2/pipe2_02.c? ? | 69 +++++++++++++++++++
> ?.../kernel/syscalls/pipe2/pipe2_02_child.c? ? | 26 +++++++
> ?4 files changed, 98 insertions(+)
> ?create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02.c
> ?create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_02_child.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 79b671d50..44254d7da 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -911,6 +911,7 @@ pipe12 pipe12
> ?pipe13 pipe13
>
> ?pipe2_01 pipe2_01
> +pipe2_02 pipe2_02
>
> ?pivot_root01 pivot_root01
>
> diff --git a/testcases/kernel/syscalls/pipe2/.gitignore
> b/testcases/kernel/syscalls/pipe2/.gitignore
> index 42350bbdc..786222de2 100644
> --- a/testcases/kernel/syscalls/pipe2/.gitignore
> +++ b/testcases/kernel/syscalls/pipe2/.gitignore
> @@ -1 +1,3 @@
> ?/pipe2_01
> +/pipe2_02
> +/pipe2_02_child
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c
> b/testcases/kernel/syscalls/pipe2/pipe2_02.c
> new file mode 100644
> index 000000000..743d78c58
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_02.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>>
> + *
> + * This case is designed to test the basic functionality about the
> + * O_CLOEXEC flag of pipe2.
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include "lapi/fcntl.h"
> +#include "tst_test.h"
> +
> +#define TESTBIN "pipe2_02_child"
> +static int fds[2];
> +
> +static void cleanup(void)
> +{
> +? ? ? ?if (fds[0] > 0)
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[0]);
> +? ? ? ?if (fds[1] > 0)
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[1]);
> +}
> +
> +static void verify_pipe2(void)
> +{
> +? ? ? ?int pid, status;
> +? ? ? ?char buf[20];
> +
> +? ? ? ?SAFE_PIPE2(fds, O_CLOEXEC);
> +? ? ? ?sprintf(buf, "%d", fds[1]);
> +? ? ? ?pid = SAFE_FORK();
> +? ? ? ?if (pid == 0) {
> +? ? ? ? ? ? ? ?if (execlp(TESTBIN, TESTBIN, buf, NULL))
> +? ? ? ? ? ? ? ? ? ? ? ?exit(2);
>
>
> Do we really need the if() condition and exit(2)? AFAIK, the exec()
> family of functions replaces the current process image with a new
> process and returns zero if succeeded.
If failed, it will return -1. So we can catch 2(exit code) in the
following code, but SAFE_EXECLP is also ok.
>
> +? ? ? ?}
> +
> +? ? ? ?SAFE_WAIT(&status);
> +? ? ? ?if (WIFEXITED(status)) {
> +? ? ? ? ? ? ? ?switch (WEXITSTATUS(status)) {
> +? ? ? ? ? ? ? ?case 0:
> +? ? ? ? ? ? ? ? ? ? ? ?tst_res(TPASS, "test O_CLOEXEC for pipe2
> success");
> +? ? ? ? ? ? ? ?break;
> +? ? ? ? ? ? ? ?case 1:
> +? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL, "test O_CLOEXEC for pipe2
> failed");
> +? ? ? ? ? ? ? ?break;
> +? ? ? ? ? ? ? ?default:
> +? ? ? ? ? ? ? ? ? ? ? ?tst_brk(TBROK, "execlp() failed");
> +? ? ? ? ? ? ? ?}
> +? ? ? ?} else {
> +? ? ? ? ? ? ? ?tst_brk(TBROK, "%s exits with unexpected error",
> TESTBIN);
> +? ? ? ?}
> +? ? ? ?cleanup();
> +}
> +
> +static const char *const resfile[] = {
> +? ? ? ?TESTBIN,
> +? ? ? ?NULL,
> +};
> +
> +static struct tst_test test = {
> +? ? ? ?.resource_files = resfile,
> +? ? ? ?.cleanup = cleanup,
> +? ? ? ?.forks_child = 1,
> +? ? ? ?.needs_root = 1,
> +? ? ? ?.test_all = verify_pipe2,
> +};
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> new file mode 100644
> index 000000000..d5ed68cf7
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_02_child.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +
> +int main(int argc, char **argv)
> +{
> +? ? ? ?int ret;
> +? ? ? ?int fd;
> +
> +? ? ? ?if (argc != 2) {
> +? ? ? ? ? ? ? ?fprintf(stderr, "Only two arguments: %s <fd>\n",
> argv[0]);
> +? ? ? ? ? ? ? ?exit(1);
> +? ? ? ?}
> +
> +? ? ? ?fd = atoi(argv[1]);
> +? ? ? ?ret = write(fd, "x", 1);
> +
> +? ? ? ?return ret != -1;
>
>
> To check if the return value equals -1 maybe not a good idea to confirm
> the 'fd' is closed. That only proof it failed to write "x" to the fd, we
> are not sure if that exists?other errors.
>
> What about using the fcntl() in the fd status check?
>
> ? ? if (fcntl(fd, F_GETFL) < 0 && errno == EBADF)
> ? ? ? ? ? ? return 0;
>
Looks good to me.
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-16 7:29 ` [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag Yang Xu
2020-04-17 9:44 ` Li Wang
@ 2020-04-21 9:53 ` Li Wang
2020-04-21 10:21 ` Yang Xu
1 sibling, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-21 9:53 UTC (permalink / raw)
To: ltp
On Thu, Apr 16, 2020 at 3:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
> runtest/syscalls | 1 +
> testcases/kernel/syscalls/pipe2/.gitignore | 1 +
> testcases/kernel/syscalls/pipe2/pipe2_03.c | 157 +++++++++++++++++++++
> 3 files changed, 159 insertions(+)
> create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 44254d7da..4d8ebc5a3 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -912,6 +912,7 @@ pipe13 pipe13
>
> pipe2_01 pipe2_01
> pipe2_02 pipe2_02
> +pipe2_03 pipe2_03
>
> pivot_root01 pivot_root01
>
> diff --git a/testcases/kernel/syscalls/pipe2/.gitignore
> b/testcases/kernel/syscalls/pipe2/.gitignore
> index 786222de2..4cc5acaf1 100644
> --- a/testcases/kernel/syscalls/pipe2/.gitignore
> +++ b/testcases/kernel/syscalls/pipe2/.gitignore
> @@ -1,3 +1,4 @@
> /pipe2_01
> /pipe2_02
> /pipe2_02_child
> +/pipe2_03
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_03.c
> b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> new file mode 100644
> index 000000000..0b5d37dd0
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> + *
> + * This case is designed to test the basic functionality about the
> + * O_DIRECT flag of pipe2.
> + *
> + * It includes three sub tests.
> + * 1) Each write(2) to the pipe is dealt with as a separate packet, and
> + * read(2)s from the pipe will read one packet at a time.
> + * 2) Writes of greater than PIPE_BUF bytes (see pipe(7)) will be split
> + * into multiple packet.
> + * 3)If a read(2) specifies a buffer size that is smaller than the next
> + * packet, then the requested number of bytes are read, and the excess
> + * bytes in the packet are discarded.
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <linux/limits.h>
> +#include "lapi/fcntl.h"
> +#include "tst_test.h"
> +
> +static int fds[2], packet_num, pipe_size;
> +static char *wrbuf;
> +static char *rdbuf;
> +static void check_peer_rw(void);
> +static void check_split(void);
> +static void check_discard(void);
> +
> +static void (*test_func[])(void) = {check_peer_rw, check_split,
> check_discard};
> +
> +static void check_peer_rw(void)
> +{
> + int i, pid;
> +
> + SAFE_PIPE2(fds, O_DIRECT | O_NONBLOCK);
> + for (i = 0; i < packet_num; i++)
> + SAFE_WRITE(1, fds[1], "x", 1);
> +
> + TEST(write(fds[1], "x", 1));
> + if (TST_RET != -1) {
> + tst_res(TFAIL, "write succeeded unexpectedly");
> + } else {
> + if (TST_ERR == EAGAIN)
> + tst_res(TPASS, "Each write(2) uses a separate
> packet");
> + else
> + tst_res(TFAIL | TTERRNO, "write failed, expected
> EAGAIN but got");
> + }
> + pid = SAFE_FORK();
> + if (!pid) {
> + memset(rdbuf, 0, pipe_size);
> + for (i = 0; i < packet_num; i++) {
> + TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
> + if (TST_RET != 1)
> + tst_res(TFAIL,
> + "Each read(2) doesn't read a
> separate packet, return %ld", TST_RET);
> + }
> + tst_res(TPASS, "Each read(2) reads a separate packet");
>
I would add _exit(0); at the end of the child.
+ _exit(0);
+ }
>
And move the tst_reap_children() from the main process to here.
+ tst_reap_children();
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
> +}
> +
> +static void check_split(void)
> +{
> + int i, pid;
> +
> + SAFE_PIPE2(fds, O_DIRECT);
> + SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF * 2);
> +
> + pid = SAFE_FORK();
> + if (!pid) {
> + memset(rdbuf, 0, pipe_size);
> + for (i = 0; i < 2; i++) {
> + TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
> + if (TST_RET != PIPE_BUF)
> + tst_res(TFAIL,
> + "write(higner than PIPE_BUF) split
> into multiple packet, return %ld", TST_RET);
> + }
> + tst_res(TPASS, "write(higner than PIPE_BUF) split into
> multiple packet");
>
+ _exit(0);
+ }
>
+ tst_reap_children();
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
> +}
> +
> +static void check_discard(void)
> +{
> + int pid;
> + char tmp_buf[20];
> + char tmp_secondbuf[20];
> +
> + SAFE_PIPE2(fds, O_DIRECT);
> + SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF);
> + SAFE_WRITE(1, fds[1], "1", 1);
> +
> + pid = SAFE_FORK();
> + if (!pid) {
> + TEST(SAFE_READ(0, fds[0], tmp_buf, 20));
> + if (TST_RET != 20)
> + tst_res(TFAIL,
> + "the excess bytes in the packet isn't
> discarded by read, return %ld", TST_RET);
> + TEST(SAFE_READ(0, fds[0], tmp_secondbuf, 20));
> + if (TST_RET == 1) {
> + if (!strcmp(tmp_secondbuf, "1"))
> + tst_res(TPASS,
> + "the excess bytes in the packet is
> discarded by read, only read 1");
> + else
> + tst_res(TFAIL,
> + "the excess bytes in the packet is
> discarded by read, expect 1 got %s", tmp_secondbuf);
> + }
>
+ _exit(0);
> + }
>
+ tst_reap_children();
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
+}
> +
> +static void verify_pipe2(unsigned int n)
> +{
> + int pid;
> +
> + pid = SAFE_FORK();
>
Why we need fork() here? I don't see any necessary reason for twice fork in
the main process.
> + if (pid == 0) {
> + (*test_func[n])();
>
> + tst_reap_children();
> + SAFE_CLOSE(fds[0]);
> + SAFE_CLOSE(fds[1]);
>
I tend to move the cloese(fd) to the test process to nearby pipe2().
Otherwise, it causes an unclear error like:
# ./pipe2_03
tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
pipe2_03.c:48: PASS: Each write(2) uses a separate packet
pipe2_03.c:61: PASS: Each read(2) reads a separate packet
pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into multiple packet
pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by read,
expect 1 got 1%A
> + }
> + tst_reap_children();
> +}
> +
> +static void setup(void)
> +{
> + SAFE_PIPE2(fds, O_DIRECT);
> + pipe_size = SAFE_FCNTL(fds[1], F_GETPIPE_SZ);
> + wrbuf = SAFE_MALLOC(PIPE_BUF * 2);
> + rdbuf = SAFE_MALLOC(pipe_size);
> + memset(wrbuf, 'x', PIPE_BUF * 2);
> + packet_num = pipe_size / PIPE_BUF;
> + SAFE_CLOSE(fds[0]);
> + SAFE_CLOSE(fds[1]);
> +}
> +
> +static void cleanup(void)
> +{
> + if (fds[0] > 0)
> + SAFE_CLOSE(fds[0]);
> + if (fds[1] > 0)
> + SAFE_CLOSE(fds[1]);
> + if (wrbuf)
> + free(wrbuf);
> + if (rdbuf)
> + free(rdbuf);
> +}
> +
> +static struct tst_test test = {
> + .setup = setup,
> + .cleanup = cleanup,
> + .forks_child = 1,
> + .test = verify_pipe2,
> + .tcnt = ARRAY_SIZE(test_func),
> +};
> --
> 2.23.0
>
>
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200421/a4e2315d/attachment-0001.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag
2020-04-21 9:41 ` Yang Xu
@ 2020-04-21 9:59 ` Li Wang
0 siblings, 0 replies; 25+ messages in thread
From: Li Wang @ 2020-04-21 9:59 UTC (permalink / raw)
To: ltp
Hi Xu,
Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:
...
> > +static void verify_pipe2(void)
> > +{
> > + int pid, status;
> > + char buf[20];
> > +
> > + SAFE_PIPE2(fds, O_CLOEXEC);
> > + sprintf(buf, "%d", fds[1]);
> > + pid = SAFE_FORK();
> > + if (pid == 0) {
> > + if (execlp(TESTBIN, TESTBIN, buf, NULL))
> > + exit(2);
> >
> >
> > Do we really need the if() condition and exit(2)? AFAIK, the exec()
> > family of functions replaces the current process image with a new
> > process and returns zero if succeeded.
> If failed, it will return -1. So we can catch 2(exit code) in the
> following code, but SAFE_EXECLP is also ok.
>
Since we have do check the status of children via WIFEXITED(status), so
even failed with -1, that also should be caught. Anyway, SAFE_EXECLP()
should work for us.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200421/a05de910/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-21 9:53 ` Li Wang
@ 2020-04-21 10:21 ` Yang Xu
2020-04-22 6:05 ` Li Wang
2020-04-22 7:29 ` Li Wang
0 siblings, 2 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-21 10:21 UTC (permalink / raw)
To: ltp
Hi Li
>
>
> On Thu, Apr 16, 2020 at 3:29 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>>
> ---
> ?runtest/syscalls? ? ? ? ? ? ? ? ? ? ? ? ? ?|? ?1 +
> ?testcases/kernel/syscalls/pipe2/.gitignore |? ?1 +
> ?testcases/kernel/syscalls/pipe2/pipe2_03.c | 157 +++++++++++++++++++++
> ?3 files changed, 159 insertions(+)
> ?create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 44254d7da..4d8ebc5a3 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -912,6 +912,7 @@ pipe13 pipe13
>
> ?pipe2_01 pipe2_01
> ?pipe2_02 pipe2_02
> +pipe2_03 pipe2_03
>
> ?pivot_root01 pivot_root01
>
> diff --git a/testcases/kernel/syscalls/pipe2/.gitignore
> b/testcases/kernel/syscalls/pipe2/.gitignore
> index 786222de2..4cc5acaf1 100644
> --- a/testcases/kernel/syscalls/pipe2/.gitignore
> +++ b/testcases/kernel/syscalls/pipe2/.gitignore
> @@ -1,3 +1,4 @@
> ?/pipe2_01
> ?/pipe2_02
> ?/pipe2_02_child
> +/pipe2_03
> diff --git a/testcases/kernel/syscalls/pipe2/pipe2_03.c
> b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> new file mode 100644
> index 000000000..0b5d37dd0
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>>
> + *
> + * This case is designed to test the basic functionality about the
> + * O_DIRECT flag of pipe2.
> + *
> + * It includes three sub tests.
> + * 1) Each write(2) to the pipe is dealt with as a separate packet, and
> + * read(2)s from the pipe will read one packet at a time.
> + * 2) Writes of greater than PIPE_BUF bytes (see pipe(7)) will be split
> + * into multiple packet.
> + * 3)If a read(2) specifies a buffer size that is smaller than the next
> + * packet, then the requested number of bytes are read, and the excess
> + * bytes in the packet are discarded.
> + */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <linux/limits.h>
> +#include "lapi/fcntl.h"
> +#include "tst_test.h"
> +
> +static int fds[2], packet_num, pipe_size;
> +static char *wrbuf;
> +static char *rdbuf;
> +static void check_peer_rw(void);
> +static void check_split(void);
> +static void check_discard(void);
> +
> +static void (*test_func[])(void) = {check_peer_rw, check_split,
> check_discard};
> +
> +static void check_peer_rw(void)
> +{
> +? ? ? ?int i, pid;
> +
> +? ? ? ?SAFE_PIPE2(fds, O_DIRECT | O_NONBLOCK);
> +? ? ? ?for (i = 0; i < packet_num; i++)
> +? ? ? ? ? ? ? ?SAFE_WRITE(1, fds[1], "x", 1);
> +
> +? ? ? ?TEST(write(fds[1], "x", 1));
> +? ? ? ?if (TST_RET != -1) {
> +? ? ? ? ? ? ? ?tst_res(TFAIL, "write succeeded unexpectedly");
> +? ? ? ?} else {
> +? ? ? ? ? ? ? ?if (TST_ERR == EAGAIN)
> +? ? ? ? ? ? ? ? ? ? ? ?tst_res(TPASS, "Each write(2) uses a
> separate packet");
> +? ? ? ? ? ? ? ?else
> +? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL | TTERRNO, "write failed,
> expected EAGAIN but got");
> +? ? ? ?}
> +? ? ? ?pid = SAFE_FORK();
> +? ? ? ?if (!pid) {
> +? ? ? ? ? ? ? ?memset(rdbuf, 0, pipe_size);
> +? ? ? ? ? ? ? ?for (i = 0; i < packet_num; i++) {
> +? ? ? ? ? ? ? ? ? ? ? ?TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
> +? ? ? ? ? ? ? ? ? ? ? ?if (TST_RET != 1)
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL,
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Each read(2) doesn't read a
> separate packet, return %ld", TST_RET);
> +? ? ? ? ? ? ? ?}
> +? ? ? ? ? ? ? ?tst_res(TPASS, "Each read(2) reads a separate packet");
>
>
> I would add _exit(0); at the end of the child.
> + ? ? ? ? ? ? ? _exit(0);
>
> +? ? ? ?}
>
>
> And move the tst_reap_children() from the main process to here.
>
> + ? ? ? tst_reap_children();
> + ? ? ? SAFE_CLOSE(fds[0]);
> + ? ? ? SAFE_CLOSE(fds[1]);
>
> +}
> +
> +static void check_split(void)
> +{
> +? ? ? ?int i, pid;
> +
> +? ? ? ?SAFE_PIPE2(fds, O_DIRECT);
> +? ? ? ?SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF * 2);
> +
> +? ? ? ?pid = SAFE_FORK();
> +? ? ? ?if (!pid) {
> +? ? ? ? ? ? ? ?memset(rdbuf, 0, pipe_size);
> +? ? ? ? ? ? ? ?for (i = 0; i < 2; i++) {
> +? ? ? ? ? ? ? ? ? ? ? ?TEST(SAFE_READ(0, fds[0], rdbuf, pipe_size));
> +? ? ? ? ? ? ? ? ? ? ? ?if (TST_RET != PIPE_BUF)
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL,
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"write(higner than PIPE_BUF)
> split into multiple packet, return %ld", TST_RET);
> +? ? ? ? ? ? ? ?}
> +? ? ? ? ? ? ? ?tst_res(TPASS, "write(higner than PIPE_BUF) split
> into multiple packet");
>
> + ? ? ? ? ? ? ? _exit(0);
>
> +? ? ? ?}
>
> + ? ? ? tst_reap_children();
> + ? ? ? SAFE_CLOSE(fds[0]);
> + ? ? ? SAFE_CLOSE(fds[1]);
>
> +}
> +
> +static void check_discard(void)
> +{
> +? ? ? ?int pid;
> +? ? ? ?char tmp_buf[20];
> +? ? ? ?char tmp_secondbuf[20];
> +
> +? ? ? ?SAFE_PIPE2(fds, O_DIRECT);
> +? ? ? ?SAFE_WRITE(1, fds[1], wrbuf, PIPE_BUF);
> +? ? ? ?SAFE_WRITE(1, fds[1], "1", 1);
> +
> +? ? ? ?pid = SAFE_FORK();
> +? ? ? ?if (!pid) {
> +? ? ? ? ? ? ? ?TEST(SAFE_READ(0, fds[0], tmp_buf, 20));
> +? ? ? ? ? ? ? ?if (TST_RET != 20)
> +? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL,
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"the excess bytes in the packet
> isn't discarded by read, return %ld", TST_RET);
> +? ? ? ? ? ? ? ?TEST(SAFE_READ(0, fds[0], tmp_secondbuf, 20));
> +? ? ? ? ? ? ? ?if (TST_RET == 1) {
> +? ? ? ? ? ? ? ? ? ? ? ?if (!strcmp(tmp_secondbuf, "1"))
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tst_res(TPASS,
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"the excess bytes in the
> packet is discarded by read, only read 1");
> +? ? ? ? ? ? ? ? ? ? ? ?else
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL,
> +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"the excess bytes in the
> packet is discarded by read, expect 1 got %s", tmp_secondbuf);
> +? ? ? ? ? ? ? ?}
>
> + ? ? ? ? ? ? ? _exit(0);
>
> +? ? ? ?}
>
> + ? ? ? tst_reap_children();
> + ? ? ? SAFE_CLOSE(fds[0]);
> + ? ? ? SAFE_CLOSE(fds[1]);
>
> +}
> +
> +static void verify_pipe2(unsigned int n)
> +{
> +? ? ? ?int pid;
> +
> +? ? ? ?pid = SAFE_FORK();
>
>
> Why we need fork() here? I don't see any necessary reason for twice fork
> in the main process.Yes, it only needs one fork. At the beginning, I think we should do
actual test in child process( I remember having such rule) especially
when there are multiple subtests. So I fork twice to make sure all sub
test in child or grandchild process.
>
> +? ? ? ?if (pid == 0) {
> +? ? ? ? ? ? ? ?(*test_func[n])();
>
> +? ? ? ? ? ? ? ?tst_reap_children();
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[0]);
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[1]);
>
>
> I tend to move the cloese(fd) to the test process to nearby?pipe2().
> Otherwise, it causes an unclear error like:
>
> # ./pipe2_03
> tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
> pipe2_03.c:48: PASS: Each write(2) uses a separate packet
> pipe2_03.c:61: PASS: Each read(2) reads a separate packet
> pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into multiple packet
> pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by
> read, expect 1 got 1%A
Even I use one fork and exit(0), I don't meet this problem. Or, I miss
something?
--- a/testcases/kernel/syscalls/pipe2/pipe2_03.c
+++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
@@ -59,6 +59,7 @@ static void check_peer_rw(void)
"Each read(2) doesn't read a
separate packet, return %ld", TST_RET);
}
tst_res(TPASS, "Each read(2) reads a separate packet");
+ _exit(0);
}
}
@@ -79,6 +80,7 @@ static void check_split(void)
"write(higner than PIPE_BUF)
split into multiple packet, return %ld", TST_RET);
}
tst_res(TPASS, "write(higner than PIPE_BUF) split into
multiple packet");
+ _exit(0);
}
}
@@ -107,21 +109,17 @@ static void check_discard(void)
tst_res(TFAIL,
"the excess bytes in the packet
is discarded by read, expect 1 got %s", tmp_secondbuf);
}
+ _exit(0);
}
}
static void verify_pipe2(unsigned int n)
{
- int pid;
-
- pid = SAFE_FORK();
- if (pid == 0) {
- (*test_func[n])();
- tst_reap_children();
- SAFE_CLOSE(fds[0]);
- SAFE_CLOSE(fds[1]);
- }
+ (*test_func[n])();
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
tst_reap_children();
+
}
>
> +? ? ? ?}
> +? ? ? ?tst_reap_children();
> +}
> +
> +static void setup(void)
> +{
> +? ? ? ?SAFE_PIPE2(fds, O_DIRECT);
> +? ? ? ?pipe_size = SAFE_FCNTL(fds[1], F_GETPIPE_SZ);
> +? ? ? ?wrbuf = SAFE_MALLOC(PIPE_BUF * 2);
> +? ? ? ?rdbuf = SAFE_MALLOC(pipe_size);
> +? ? ? ?memset(wrbuf, 'x', PIPE_BUF * 2);
> +? ? ? ?packet_num = pipe_size / PIPE_BUF;
> +? ? ? ?SAFE_CLOSE(fds[0]);
> +? ? ? ?SAFE_CLOSE(fds[1]);
> +}
> +
> +static void cleanup(void)
> +{
> +? ? ? ?if (fds[0] > 0)
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[0]);
> +? ? ? ?if (fds[1] > 0)
> +? ? ? ? ? ? ? ?SAFE_CLOSE(fds[1]);
> +? ? ? ?if (wrbuf)
> +? ? ? ? ? ? ? ?free(wrbuf);
> +? ? ? ?if (rdbuf)
> +? ? ? ? ? ? ? ?free(rdbuf);
> +}
> +
> +static struct tst_test test = {
> +? ? ? ?.setup = setup,
> +? ? ? ?.cleanup = cleanup,
> +? ? ? ?.forks_child = 1,
> +? ? ? ?.test = verify_pipe2,
> +? ? ? ?.tcnt = ARRAY_SIZE(test_func),
> +};
> --
> 2.23.0
>
>
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
>
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-21 10:21 ` Yang Xu
@ 2020-04-22 6:05 ` Li Wang
2020-04-22 6:14 ` Yang Xu
2020-04-22 7:29 ` Li Wang
1 sibling, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-22 6:05 UTC (permalink / raw)
To: ltp
Xu,
On Tue, Apr 21, 2020 at 6:24 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> ...
> >
> > + if (pid == 0) {
> > + (*test_func[n])();
> >
> > + tst_reap_children();
> > + SAFE_CLOSE(fds[0]);
> > + SAFE_CLOSE(fds[1]);
> >
> >
> > I tend to move the cloese(fd) to the test process to nearby pipe2().
> > Otherwise, it causes an unclear error like:
> >
> > # ./pipe2_03
> > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
> > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
> > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
> > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into multiple
> packet
> > pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by
> > read, expect 1 got 1%A
> Even I use one fork and exit(0), I don't meet this problem. Or, I miss
> something?
>
My test env is kernel-5.7.0-rc1+ x86_64, I haven't figured out why the
read() get more words from the pipe, but the error has gone one I move the
safe_close(fd) part to the test main process.
> --- a/testcases/kernel/syscalls/pipe2/pipe2_03.c
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -59,6 +59,7 @@ static void check_peer_rw(void)
> "Each read(2) doesn't read a
> separate packet, return %ld", TST_RET);
> }
> tst_res(TPASS, "Each read(2) reads a separate packet");
> + _exit(0);
> }
> }
>
> @@ -79,6 +80,7 @@ static void check_split(void)
> "write(higner than PIPE_BUF)
> split into multiple packet, return %ld", TST_RET);
> }
> tst_res(TPASS, "write(higner than PIPE_BUF) split into
> multiple packet");
> + _exit(0);
> }
> }
>
> @@ -107,21 +109,17 @@ static void check_discard(void)
> tst_res(TFAIL,
> "the excess bytes in the packet
> is discarded by read, expect 1 got %s", tmp_secondbuf);
> }
> + _exit(0);
> }
> }
>
> static void verify_pipe2(unsigned int n)
> {
> - int pid;
> -
> - pid = SAFE_FORK();
> - if (pid == 0) {
> - (*test_func[n])();
> - tst_reap_children();
> - SAFE_CLOSE(fds[0]);
> - SAFE_CLOSE(fds[1]);
> - }
> + (*test_func[n])();
> + SAFE_CLOSE(fds[0]);
> + SAFE_CLOSE(fds[1]);
> tst_reap_children();
>
In my opinion, it is not a good habit to wait for children's status after
the test function, we'd better do that at the behind of fork() in the test
main process. I'd only keep the (*test_func[n])(); in this verify_pipe2()
but no more else.
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200422/2144c942/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 6:05 ` Li Wang
@ 2020-04-22 6:14 ` Yang Xu
0 siblings, 0 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-22 6:14 UTC (permalink / raw)
To: ltp
Hi Li
> Xu,
>
> On Tue, Apr 21, 2020 at 6:24 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>
> ...
> >
> >? ? ?+? ? ? ?if (pid == 0) {
> >? ? ?+? ? ? ? ? ? ? ?(*test_func[n])();
> >
> >? ? ?+? ? ? ? ? ? ? ?tst_reap_children();
> >? ? ?+? ? ? ? ? ? ? ?SAFE_CLOSE(fds[0]);
> >? ? ?+? ? ? ? ? ? ? ?SAFE_CLOSE(fds[1]);
> >
> >
> > I tend to move the cloese(fd) to the test process to nearby?pipe2().
> > Otherwise, it causes an unclear error like:
> >
> > # ./pipe2_03
> > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
> > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
> > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
> > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into
> multiple packet
> > pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by
> > read, expect 1 got 1%A
> Even I use one fork and exit(0), I don't meet this problem. Or, I miss
> something?
>
>
> My test env is kernel-5.7.0-rc1+ x86_64, I haven't figured out why the
> read() get more words from the pipe, but the error has gone one I move
> the safe_close(fd) part to the test main process.
>
OK.
>
> --- a/testcases/kernel/syscalls/pipe2/pipe2_03.c
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -59,6 +59,7 @@ static void check_peer_rw(void)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Each read(2) doesn't read a
> separate packet, return %ld", TST_RET);
> ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ? ?tst_res(TPASS, "Each read(2) reads a separate
> packet");
> +? ? ? ? ? ? ? ? _exit(0);
> ? ? ? ? ?}
> ? }
>
> @@ -79,6 +80,7 @@ static void check_split(void)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"write(higner than PIPE_BUF)
> split into multiple packet, return %ld", TST_RET);
> ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ? ?tst_res(TPASS, "write(higner than PIPE_BUF) split
> into
> multiple packet");
> +? ? ? ? ? ? ? ? _exit(0);
> ? ? ? ? ?}
> ? }
>
> @@ -107,21 +109,17 @@ static void check_discard(void)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tst_res(TFAIL,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"the excess bytes in the
> packet
> is discarded by read, expect 1 got %s", tmp_secondbuf);
> ? ? ? ? ? ? ? ? ?}
> +? ? ? ? ? ? ? ? _exit(0);
> ? ? ? ? ?}
> ? }
>
> ? static void verify_pipe2(unsigned int n)
> ? {
> -? ? ? ?int pid;
> -
> -? ? ? ?pid = SAFE_FORK();
> -? ? ? ?if (pid == 0) {
> -? ? ? ? ? ? ? ?(*test_func[n])();
> -? ? ? ? ? ? ? ?tst_reap_children();
> -? ? ? ? ? ? ? ?SAFE_CLOSE(fds[0]);
> -? ? ? ? ? ? ? ?SAFE_CLOSE(fds[1]);
> -? ? ? ?}
> + ?(*test_func[n])();
> +? ? ? ?SAFE_CLOSE(fds[0]);
> +? ? ? ?SAFE_CLOSE(fds[1]);
> ? ? ? ? ?tst_reap_children();
>
>
> In my opinion, it is not a good habit to wait for children's status
> after the test function, we'd better do that at the behind of fork() in
> the test main process. I'd only keep the ?(*test_func[n])(); in this
> verify_pipe2() but no more else.
Yes.
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-21 10:21 ` Yang Xu
2020-04-22 6:05 ` Li Wang
@ 2020-04-22 7:29 ` Li Wang
2020-04-22 8:08 ` Yang Xu
1 sibling, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-22 7:29 UTC (permalink / raw)
To: ltp
Hi Xu,
Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:
...
> >
> > # ./pipe2_03
> > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
> > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
> > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
> > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into multiple
> packet
> > pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by
> > read, expect 1 got 1%A
> Even I use one fork and exit(0), I don't meet this problem. Or, I miss
> something?
>
I think I got the point.
In your test code for pipe2(), you didn't use pipe in the correct way. Even
that we define the fds[2] as a global variable, there still have a new copy
for fds[2] in the child process, we should close one of the pipes in child
and parent to make the connection is built correspondingly, then that will
work normally for us.
I suggest redesigning some of the test structs. What do you think?
Fyi:
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200422/13dcd3ed/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 7:29 ` Li Wang
@ 2020-04-22 8:08 ` Yang Xu
2020-04-22 8:12 ` Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-22 8:08 UTC (permalink / raw)
To: ltp
Hi Li
> Hi Xu,
>
> Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>
> ...
> >
> > # ./pipe2_03
> > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
> > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
> > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
> > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into
> multiple packet
> > pipe2_03.c:108: FAIL: the excess bytes in the packet is discarded by
> > read, expect 1 got 1%A
> Even I use one fork and exit(0), I don't meet this problem. Or, I miss
> something?
>
>
> I think I got the point.
>
> In your test code for pipe2(), you didn't use pipe in the correct way.
> Even that we define the fds[2] as a global?variable, there still have a
> new copy for fds[2] in the child process, we should close one of the
> pipes in child and parent to make the connection is built
> correspondingly, then that will work normally for us.
>
> I suggest redesigning some of the test structs. What do you think?
Yes.
>
> Fyi:
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
In principle, we should use the following format
step1 pipe(fds);
step2 SAFE_FORK();
child process
step3 close one fd and do something
parent process
step4 close another fd and do something
Also, we should not do write opeartion in step 1~2, otherwise child
process will inherit data.
>
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 8:08 ` Yang Xu
@ 2020-04-22 8:12 ` Yang Xu
2020-04-22 9:31 ` Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-22 8:12 UTC (permalink / raw)
To: ltp
Hi Li
> Hi Li
>
>> Hi Xu,
>>
>> Yang Xu <xuyang2018.jy@cn.fujitsu.com
>> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>>
>> ??? ...
>> ???? >
>> ???? > # ./pipe2_03
>> ???? > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
>> ???? > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
>> ???? > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
>> ???? > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into
>> ??? multiple packet
>> ???? > pipe2_03.c:108: FAIL: the excess bytes in the packet is
>> discarded by
>> ???? > read, expect 1 got 1%A
>> ??? Even I use one fork and exit(0), I don't meet this problem. Or, I
>> miss
>> ??? something?
>>
>>
>> I think I got the point.
>>
>> In your test code for pipe2(), you didn't use pipe in the correct way.
>> Even that we define the fds[2] as a global?variable, there still have
>> a new copy for fds[2] in the child process, we should close one of the
>> pipes in child and parent to make the connection is built
>> correspondingly, then that will work normally for us.
>>
>> I suggest redesigning some of the test structs. What do you think?
> Yes.
>>
>> Fyi:
>> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
BTW, pipe02.c can use SAFE_PIPE macro and remove UCLINUX ifdef.
>>
> In principle, we should use the following format
> step1????? pipe(fds);
> step2????? SAFE_FORK();
> ????? child process
> step3???????? close one fd and do something
> ????? parent process
> step4???????? close another fd and do something
>
> Also, we should not do write opeartion in step 1~2, otherwise child
> process will inherit data.
>>
>> --
>> Regards,
>> Li Wang
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 8:12 ` Yang Xu
@ 2020-04-22 9:31 ` Yang Xu
2020-04-22 10:05 ` Li Wang
0 siblings, 1 reply; 25+ messages in thread
From: Yang Xu @ 2020-04-22 9:31 UTC (permalink / raw)
To: ltp
Hi Li
> Hi Li
>
>
>> Hi Li
>>
>>> Hi Xu,
>>>
>>> Yang Xu <xuyang2018.jy@cn.fujitsu.com
>>> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>>>
>>> ??? ...
>>> ???? >
>>> ???? > # ./pipe2_03
>>> ???? > tst_test.c:1246: INFO: Timeout per run is 0h 05m 00s
>>> ???? > pipe2_03.c:48: PASS: Each write(2) uses a separate packet
>>> ???? > pipe2_03.c:61: PASS: Each read(2) reads a separate packet
>>> ???? > pipe2_03.c:81: PASS: write(higner than PIPE_BUF) split into
>>> ??? multiple packet
>>> ???? > pipe2_03.c:108: FAIL: the excess bytes in the packet is
>>> discarded by
>>> ???? > read, expect 1 got 1%A
>>> ??? Even I use one fork and exit(0), I don't meet this problem. Or, I
>>> miss
>>> ??? something?
>>>
>>>
>>> I think I got the point.
>>>
>>> In your test code for pipe2(), you didn't use pipe in the correct
>>> way. Even that we define the fds[2] as a global?variable, there still
>>> have a new copy for fds[2] in the child process, we should close one
>>> of the pipes in child and parent to make the connection is built
>>> correspondingly, then that will work normally for us.
>>>
>>> I suggest redesigning some of the test structs. What do you think?
>> Yes.
>>>
>>> Fyi:
>>> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
>
> BTW, pipe02.c can use SAFE_PIPE macro and remove? UCLINUX ifdef.
>>>
>> In principle, we should use the following format
>> step1????? pipe(fds);
>> step2????? SAFE_FORK();
>> ?????? child process
>> step3???????? close one fd and do something
>> ?????? parent process
>> step4???????? close another fd and do something
>>
>> Also, we should not do write opeartion in step 1~2, otherwise child
>> process will inherit data.
modify this case ,url as below:
https://github.com/xuyang0410/ltp/commit/6e701803b39123b5abf74bd292c36fee73b54065
How about it?
ps: I meet a problem that string without the trailing string zero
terminator, so I want to use memcmp or strncmp.
>>>
>>> --
>>> Regards,
>>> Li Wang
>>
>>
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 9:31 ` Yang Xu
@ 2020-04-22 10:05 ` Li Wang
2020-04-22 10:06 ` Yang Xu
0 siblings, 1 reply; 25+ messages in thread
From: Li Wang @ 2020-04-22 10:05 UTC (permalink / raw)
To: ltp
On Wed, Apr 22, 2020 at 5:31 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:
> ...
> >>> I think I got the point.
> >>>
> >>> In your test code for pipe2(), you didn't use pipe in the correct
> >>> way. Even that we define the fds[2] as a global variable, there still
> >>> have a new copy for fds[2] in the child process, we should close one
> >>> of the pipes in child and parent to make the connection is built
> >>> correspondingly, then that will work normally for us.
> >>>
> >>> I suggest redesigning some of the test structs. What do you think?
> >> Yes.
> >>>
> >>> Fyi:
> >>>
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
> >
> > BTW, pipe02.c can use SAFE_PIPE macro and remove UCLINUX ifdef.
> >>>
> >> In principle, we should use the following format
> >> step1 pipe(fds);
> >> step2 SAFE_FORK();
> >> child process
> >> step3 close one fd and do something
> >> parent process
> >> step4 close another fd and do something
> >>
> >> Also, we should not do write opeartion in step 1~2, otherwise child
> >> process will inherit data.
> modify this case ,url as below:
>
> https://github.com/xuyang0410/ltp/commit/6e701803b39123b5abf74bd292c36fee73b54065
>
> How about it?
>
I leave comment in your patch in github. Thanks for the quick fix, the rest
LGTM.
Can you rebase the code on the latest master and format V4 patch to ML?
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200422/7623f96e/attachment.htm>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag
2020-04-22 10:05 ` Li Wang
@ 2020-04-22 10:06 ` Yang Xu
0 siblings, 0 replies; 25+ messages in thread
From: Yang Xu @ 2020-04-22 10:06 UTC (permalink / raw)
To: ltp
Hi Li
? 2020/04/22 18:05, Li Wang ??:
>
>
> On Wed, Apr 22, 2020 at 5:31 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
>
> ...
> >>> I think I got the point.
> >>>
> >>> In your test code for pipe2(), you didn't use pipe in the correct
> >>> way. Even that we define the fds[2] as a global?variable, there
> still
> >>> have a new copy for fds[2] in the child process, we should
> close one
> >>> of the pipes in child and parent to make the connection is built
> >>> correspondingly, then that will work normally for us.
> >>>
> >>> I suggest redesigning some of the test structs. What do you think?
> >> Yes.
> >>>
> >>> Fyi:
> >>>
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/pipe/pipe02.c
>
> >
> > BTW, pipe02.c can use SAFE_PIPE macro and remove? UCLINUX ifdef.
> >>>
> >> In principle, we should use the following format
> >> step1????? pipe(fds);
> >> step2????? SAFE_FORK();
> >> ?????? child process
> >> step3???????? close one fd and do something
> >> ?????? parent process
> >> step4???????? close another fd and do something
> >>
> >> Also, we should not do write opeartion in step 1~2, otherwise child
> >> process will inherit data.
> modify this case ,url as below:
> https://github.com/xuyang0410/ltp/commit/6e701803b39123b5abf74bd292c36fee73b54065
>
> How about it?
>
>
> I leave comment?in your patch in?github. Thanks for the quick fix, the
> rest LGTM.
>
> Can you rebase the code on the latest master and format V4 patch to ML?
OK. I will send a v4 about this patchset.
>
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-04-22 10:06 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-10 8:52 [LTP] [PATCH] syscalls/pipe2_01: convert into new API Yang Xu
2020-04-15 5:11 ` [LTP] [PATCH v2] " Yang Xu
2020-04-16 7:28 ` [LTP] [PATCH v3 0/3] increase pipe2 coverage Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 1/3] syscalls/pipe2_01: convert into new API Yang Xu
2020-04-21 8:19 ` Li Wang
2020-04-21 8:27 ` Yang Xu
2020-04-16 7:29 ` [LTP] [PATCH v3 2/3] syscalls/pipe2_02: Add new test for pipe2 O_CLOEXEC flag Yang Xu
2020-04-21 8:49 ` Li Wang
2020-04-21 9:41 ` Yang Xu
2020-04-21 9:59 ` Li Wang
2020-04-16 7:29 ` [LTP] [PATCH v3 3/3] syscalls/pipe2_03: Add new test for pipe2 O_DIRECT flag Yang Xu
2020-04-17 9:44 ` Li Wang
2020-04-17 9:56 ` Yang Xu
2020-04-17 10:13 ` Li Wang
2020-04-17 10:50 ` Li Wang
2020-04-21 9:53 ` Li Wang
2020-04-21 10:21 ` Yang Xu
2020-04-22 6:05 ` Li Wang
2020-04-22 6:14 ` Yang Xu
2020-04-22 7:29 ` Li Wang
2020-04-22 8:08 ` Yang Xu
2020-04-22 8:12 ` Yang Xu
2020-04-22 9:31 ` Yang Xu
2020-04-22 10:05 ` Li Wang
2020-04-22 10:06 ` Yang Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.