[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[laniel_francis]

Hi.


I added a new test file (pipe2_03.c) to test pipe2 system call with and without
O_NONBLOCK enabled.
This test uses the macro SAFE_PIPE2 that I added, this macro is just the
equivalent of SAFE_PIPE for pipe2.

I do not know if it is correct to do that but at the end of the test function we
need to set the pipe in the same state as it was at the end of setup function:
1. Set the O_NONBLOCK flag to the pipe.
2. Read the pipe to empty it.
3. We do not set its size to default because we do not care of it.
If we do not do that, there will be problem when this test is run multiple time
(this problem was found when running ./pipe2_03 -i 10).

The diff --stat output is the following:
 include/old/safe_macros.h                  |   3 +++
 include/tst_safe_macros.h                  |   6 ++++++
 lib/safe_macros.c                          |  15 ++++++++++++++
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/pipe2/.gitignore |   1 +
 testcases/kernel/syscalls/pipe2/pipe2_03.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 161 insertions(+)


Best regards.

[LTP] [PATCH v3 1/2] Add SAFE_PIPE2.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

This macro is the equivalent of SAFE_PIPE for pipe2 system call.
---
 include/old/safe_macros.h |  3 +++
 include/tst_safe_macros.h |  6 ++++++
 lib/safe_macros.c         | 15 +++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h
index e778d3077..a7bd43c80 100644
--- a/include/old/safe_macros.h
+++ b/include/old/safe_macros.h
@@ -66,6 +66,9 @@
 #define SAFE_PIPE(cleanup_fn, fildes)	\
 	safe_pipe(__FILE__, __LINE__, cleanup_fn, (fildes))
 
+#define SAFE_PIPE2(fildes, flags) \
+	safe_pipe2(__FILE__, __LINE__, NULL, (fildes), (flags))
+
 #define SAFE_READ(cleanup_fn, len_strict, fildes, buf, nbyte)	\
 	safe_read(__FILE__, __LINE__, cleanup_fn, (len_strict), (fildes), \
 	    (buf), (nbyte))
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index d95d26219..a21d75646 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -87,6 +87,12 @@ static inline int safe_dup(const char *file, const int lineno,
 #define SAFE_PIPE(fildes) \
 	safe_pipe(__FILE__, __LINE__, NULL, (fildes))
 
+int safe_pipe2(const char *file, const int lineno, void (*cleanup_fn) (void),
+	       int fildes[2], int flags);
+
+#define SAFE_PIPE2(fildes, flags) \
+	safe_pipe2(__FILE__, __LINE__, NULL, (fildes), (flags))
+
 #define SAFE_READ(len_strict, fildes, buf, nbyte) \
 	safe_read(__FILE__, __LINE__, NULL, (len_strict), (fildes), (buf), (nbyte))
 
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index dde9b7b5e..780245821 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -248,6 +248,21 @@ int safe_pipe(const char *file, const int lineno, void (*cleanup_fn) (void),
 	return rval;
 }
 
+int safe_pipe2(const char *file, const int lineno, void (*cleanup_fn) (void),
+               int fildes[2], int flags)
+{
+	int rval;
+
+	rval = pipe2(fildes, flags);
+	if (rval == -1) {
+		tst_brkm(TBROK | TERRNO, cleanup_fn,
+			 "%s:%d: pipe2({%d,%d}) failed",
+			 file, lineno, fildes[0], fildes[1]);
+	}
+
+	return rval;
+}
+
 ssize_t safe_read(const char *file, const int lineno, void (*cleanup_fn) (void),
                   char len_strict, int fildes, void *buf, size_t nbyte)
 {
-- 
2.20.1

[LTP] [PATCH v3 2/2] Add new test for pipe2 with/without O_NONBLOCK mode.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

The new test (pipe2_03.c) checks the following:
1. Create a pipe with O_NONBLOCK.
2. Check that this flag is set.
3. Check that pipe size is 16 * PAGE_SIZE.
4. Reduce pipe size to PAGE_SIZE.
5. Write buffer bigger than page size and see that second write fails.
6. Set pipe's flags to default.
7. Fork and do a write in the child, its state must be 'S' and is checked from
the father.
---
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/pipe2/.gitignore |   1 +
 testcases/kernel/syscalls/pipe2/pipe2_03.c | 135 +++++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 0ad66ca5e..24bc90472 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -909,6 +909,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 cd38bb309..01d980dba 100644
--- a/testcases/kernel/syscalls/pipe2/.gitignore
+++ b/testcases/kernel/syscalls/pipe2/.gitignore
@@ -1,2 +1,3 @@
 /pipe2_01
 /pipe2_02
+/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..8f6e372ed
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Francis Laniel. All rights reserved.
+ * Author: Francis Laniel <laniel_francis@privacyrequired.com>
+ *
+ * Test Description:
+ * This Program tests getting and setting the pipe size.
+ * It also tests what happen when you write to a full pipe depending on whether
+ * O_NONBLOCK is set or not.
+ */
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <features.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <assert.h>
+#include <sys/select.h>
+
+#include "lapi/fcntl.h"
+#include "tst_test.h"
+
+
+static int fds[2];
+static long page_size;
+
+static void setup(void)
+{
+	/*
+	 * Create the pipe with O_NONBLOCK.
+	 */
+	SAFE_PIPE2(fds, O_NONBLOCK);
+
+	/*
+	 * Get the system page size.
+	 */
+	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
+}
+
+static void test_pipe2(void)
+{
+	long flags;
+	long pipe_size;
+
+	char *buf;
+
+	pid_t pid;
+	int status;
+
+	flags = SAFE_FCNTL(fds[1], F_GETFL);
+
+	if (!(flags & O_NONBLOCK))
+		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
+
+	/*
+	 * A pipe has two file descriptors.
+	 * But in the kernel these two file descriptors point to the same pipe.
+	 * So setting size from first file handle set size for the pipe.
+	 */
+	SAFE_FCNTL(fds[0], F_SETPIPE_SZ, 0);
+
+	/*
+	 * So getting size from the second file descriptor return the size of
+	 * the pipe which was changed before with first file descriptor.
+	 */
+	pipe_size = SAFE_FCNTL(fds[1], F_GETPIPE_SZ);
+
+	if (pipe_size != page_size)
+		tst_res(TFAIL, "Pipe size (%ld) must be page size (%ld)",
+			pipe_size, page_size);
+
+	buf = alloca(page_size);
+
+	SAFE_WRITE(1, fds[1], buf, page_size);
+
+	/*
+	 * This write should return -1 and errno set to either EAGAIN or
+	 * EWOULDBLOCK because pipe is already full.
+	 */
+	if (write(fds[1], buf, page_size) != -1
+		&& (errno != EAGAIN || errno != EWOULDBLOCK))
+		tst_res(TFAIL | TERRNO, "write() succeeded and should not");
+
+	SAFE_FCNTL(fds[1], F_SETFL, flags & ~O_NONBLOCK);
+
+	flags = SAFE_FCNTL(fds[1], F_GETFL);
+
+	if (flags & O_NONBLOCK)
+		tst_res(TFAIL, "O_NONBLOCK flag must not be set.");
+
+	pid = SAFE_FORK();
+
+	/*
+	 * Since writes are now blocking the child must wait forever on this
+	 * write.
+	 */
+	if (!pid)
+		SAFE_WRITE(1, fds[1], buf, page_size);
+
+	if (TST_PROCESS_STATE_WAIT(pid, 'S', 1000))
+		tst_res(TFAIL, "Child must be stopped.");
+	else
+		tst_res(TPASS, "Child is stopped.");
+
+	SAFE_KILL(pid, SIGKILL);
+
+	SAFE_WAIT(&status);
+
+	/*
+	 * Before exiting this function we need to reset the pipe state as it
+	 * was at the end of setup function.
+	 * Otherwise there will be problem when this test is run multiple times.
+	 *
+	 * So we need to:
+	 * 1. Set the flags to O_NONBLOCK.
+	 * 2. Read the pipe to empty it.
+	 */
+	SAFE_FCNTL(fds[1], F_SETFL, flags | O_NONBLOCK);
+
+	SAFE_READ(1, fds[0], buf, page_size);
+}
+
+static void cleanup(void)
+{
+	for (int i = 0; i < 2; i++)
+		if (fds[i] > 0)
+			SAFE_CLOSE(fds[i]);
+}
+
+static struct tst_test test = {
+	.test_all = test_pipe2,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+};
\ No newline at end of file
-- 
2.20.1

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[Yang Xu]

Hi Laniel


> Hi.
> 
> 
> I added a new test file (pipe2_03.c) to test pipe2 system call with and without
> O_NONBLOCK enabled.
> This test uses the macro SAFE_PIPE2 that I added, this macro is just the
> equivalent of SAFE_PIPE for pipe2.
> 
> I do not know if it is correct to do that but at the end of the test function we
> need to set the pipe in the same state as it was at the end of setup function:
> 1. Set the O_NONBLOCK flag to the pipe.
> 2. Read the pipe to empty it.
As my previous mail said, fill up the pipe in setup, so we don't need to 
read the pipe. we only need to set the non_block flag when using -i 
parameter.

I modify your patch and merged into my ltp own fork.
see 
https://github.com/xuyang0410/ltp/commits/Laniel_francis_pipe2_03_v3.fixes

ps: I will convert pipe2_01/02.c into new libray and use SAFE_PIPE2 macro.

Best Regards
Yang Xu
> 3. We do not set its size to default because we do not care of it.
> If we do not do that, there will be problem when this test is run multiple time
> (this problem was found when running ./pipe2_03 -i 10).
> 
> The diff --stat output is the following:
>   include/old/safe_macros.h                  |   3 +++
>   include/tst_safe_macros.h                  |   6 ++++++
>   lib/safe_macros.c                          |  15 ++++++++++++++
>   runtest/syscalls                           |   1 +
>   testcases/kernel/syscalls/pipe2/.gitignore |   1 +
>   testcases/kernel/syscalls/pipe2/pipe2_03.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   6 files changed, 161 insertions(+)
> 
> 
> Best regards.
> 
> 
> 

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[Yang Xu]

Hi Li

> Hi Laniel
> 
> 
>> Hi.
>>
>>
>> I added a new test file (pipe2_03.c) to test pipe2 system call with 
>> and without
>> O_NONBLOCK enabled.
>> This test uses the macro SAFE_PIPE2 that I added, this macro is just the
>> equivalent of SAFE_PIPE for pipe2.
>>
>> I do not know if it is correct to do that but at the end of the test 
>> function we
>> need to set the pipe in the same state as it was at the end of setup 
>> function:
>> 1. Set the O_NONBLOCK flag to the pipe.
>> 2. Read the pipe to empty it.
> As my previous mail said, fill up the pipe in setup, so we don't need to 
> read the pipe. we only need to set the non_block flag when using -i 
> parameter.
> 
> I modify your patch and merged into my ltp own fork.
> see 
> https://github.com/xuyang0410/ltp/commits/Laniel_francis_pipe2_03_v3.fixes
> 
> ps: I will convert pipe2_01/02.c into new libray and use SAFE_PIPE2 macro.
Can you merge the safe_pipe2 patch(don't need to add this macro into old 
safe_macro header) into master? so then I can write new test cases 
(using this macro) to test O_DIRECT and  O_CLOEXEC flag function.

Also, feel free to comment the Laniel_francis_pipe2_03_v3.fixes branch 
(laniel write this case to test O_NONBLOCK mode, it is similar with 
vmsplice04.c).

Best Regards
Yang Xu
> 
> Best Regards
> Yang Xu
>> 3. We do not set its size to default because we do not care of it.
>> If we do not do that, there will be problem when this test is run 
>> multiple time
>> (this problem was found when running ./pipe2_03 -i 10).
>>
>> The diff --stat output is the following:
>> ? include/old/safe_macros.h????????????????? |?? 3 +++
>> ? include/tst_safe_macros.h????????????????? |?? 6 ++++++
>> ? lib/safe_macros.c????????????????????????? |? 15 ++++++++++++++
>> ? runtest/syscalls?????????????????????????? |?? 1 +
>> ? testcases/kernel/syscalls/pipe2/.gitignore |?? 1 +
>> ? testcases/kernel/syscalls/pipe2/pipe2_03.c | 135 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 
>>
>> ? 6 files changed, 161 insertions(+)
>>
>>
>> Best regards.
>>
>>
>>

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[Li Wang]

Hi Xu,

Thanks for reviewing the patches.

Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:

> ...
> > I modify your patch and merged into my ltp own fork.
> > see
> >
> https://github.com/xuyang0410/ltp/commits/Laniel_francis_pipe2_03_v3.fixes
> >
> > ps: I will convert pipe2_01/02.c into new libray and use SAFE_PIPE2
> macro.
> Can you merge the safe_pipe2 patch(don't need to add this macro into old
> safe_macro header) into master? so then I can write new test cases
>

I'm ok with only support it in the new library.

But if we export the safe macro only to use by new API, we'd better add it
into lib/tst_safe_macros.c
but not lib/safe_macros.c, this patch leaves the 'void (*cleanup_fn)
(void)' hook in function safe_pipe2()
and put in safe_macros.c that is compatible with both API using.

Do you think so? if yes I can help to move it to tst_safe_macros.c and
apply it.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200417/1982f0dc/attachment-0001.htm>

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[Yang Xu]

Hi Li


> Hi Xu,
> 
> Thanks for reviewing the patches.
> 
> Yang Xu <xuyang2018.jy@cn.fujitsu.com 
> <mailto:xuyang2018.jy@cn.fujitsu.com>> wrote:
> 
>     ...
>      > I modify your patch and merged into my ltp own fork.
>      > see
>      >
>     https://github.com/xuyang0410/ltp/commits/Laniel_francis_pipe2_03_v3.fixes
>      >
>      > ps: I will convert pipe2_01/02.c into new libray and use
>     SAFE_PIPE2 macro.
>     Can you merge the safe_pipe2 patch(don't need to add this macro into
>     old
>     safe_macro header) into master? so then I can write new test cases
> 
> 
> I'm ok with only support it in the new library.
> 
> But if we export the safe macro only to use by new API, we'd better add 
> it into lib/tst_safe_macros.c
> but not lib/safe_macros.c, this patch leaves the 'void (*cleanup_fn) 
> (void)'?hook in function safe_pipe2()
> and put in safe_macros.c that is compatible?with both API using.
> 
> Do you think so? if yes I can help to move it to?tst_safe_macros.c and 
> apply it.
Yes. I only noticed header file place and forgot C file place.
> 
> -- 
> Regards,
> Li Wang

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[Li Wang]

Hi Xu, Laniel,

Patchset changed as the following and pushed:

  * [1/2] move safe_pipe2() to tst_safe_macro.c
  * [1/2] remove cleanup_fn callback from function
  * [1/2] replace tst_brk() by tst_brk_()
  * [1/2] add flag in the print message
  * [2/2] rename to pipe2_04.c
  * [2/2] remove useless status variable
  * [2/2] code indent cleanup

@Laniel, remember to use '-s' in the git-commit command next time, then it
will help add the Signed-off-by: automatically.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200417/19996939/attachment-0001.htm>

[LTP] Add new test for pipe2 with/without O_NONBLOCK mode

[laniel_francis]

I added a new test file (pipe2_03.c) to test pipe2 system call with and without
O_NONBLOCK enabled.
This test uses the macro SAFE_PIPE2 that I added, this macro is just the
equivalent of SAFE_PIPE for pipe2.

The diff --stat output is the following:
include/safe_macros_fn.h                   |   3 +++
include/tst_safe_macros.h                  |   3 +++
lib/safe_macros.c                          |  15 ++++++++++++++
testcases/kernel/syscalls/pipe2/.gitignore |   1 +
testcases/kernel/syscalls/pipe2/pipe2_03.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 152 insertions(+)


Best regards.