[LTP] [PATCH] Increase pipe2() coverage.

[LTP] [PATCH] Increase pipe2() coverage.

[laniel_francis]

From: Francis Laniel <laniel_francis@privacyrequired.com>

A new test was added (pipe2_03.c), this test checks the following:
1. Create a pipe with O_NONBLOCK.
2. Check that this flag is set.
3. Check that pipe size is 65536B.
4. Reduce pipe size to 4096B.
5. Write buffer bigger than page size and see that second write fails.
6. Set pipe's flags to default.
7. Check that pipe is still full with select.
---
 testcases/kernel/syscalls/pipe2/.gitignore |   1 +
 testcases/kernel/syscalls/pipe2/pipe2_03.c | 215 +++++++++++++++++++++
 2 files changed, 216 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c

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..ea225fddc
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Francis Laniel, 2020                                         */
+/******************************************************************************/
+/******************************************************************************/
+/*                                                                            */
+/* File:        pipe2_03.c                                                    */
+/*                                                                            */
+/* 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 or not.                    */
+/*                                                                            */
+/* Usage:  <for command-line>                                                 */
+/* pipe2_03 [-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_03                                                      */
+/*                                                                            */
+/* Author:      Francis Laniel                                                */
+/*                                                                            */
+/* History:     Created - Mar 28 2020 - Francis Laniel                        */
+/******************************************************************************/
+#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 "tst_test.h"
+
+#define PAGE_NR 16
+#define READ_SIDE 0
+#define WRITE_SIDE 1
+#define SECONDS 3
+#define MICROSECONDS 0
+
+/**
+ * The two file descriptors of the pipe.
+ */
+static int fds[2];
+
+/**
+ * This variable will contain the system page size after setup.
+ */
+static long page_size;
+
+/**
+ * Setup everything.
+ * This function will:
+ * 1. Create the pipe with O_NONBLOCK.
+ * 2. Get system page size with sysconf().
+ */
+static void setup(void)
+{
+	/*
+	 * Check that Linux version is greater than 2.6.35 to be able to use
+	 * F_GETPIPE_SZ and F_SETPIPE_SZ.
+	 */
+	if (tst_kvercmp(2, 6, 35) < 0)
+		tst_brk(TCONF, "This test can only run on kernels that are 2.6.35 and higher");
+
+	/*
+	 * Create the pip with O_NONBLOCK.
+	 */
+	if (pipe2(fds, O_NONBLOCK))
+		tst_brk(TBROK | TERRNO, "pipe2() failed");
+
+	/*
+	 * Get the system page size.
+	 */
+	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
+}
+
+/**
+ * Test everything.
+ */
+static void test_pipe2(void)
+{
+	int ret;
+
+	long flags;
+	long pipe_size;
+
+	char *buf;
+
+	fd_set set;
+	struct timeval timeout;
+
+	/*
+	 * Get the flags of the pipe.
+	 */
+	flags = SAFE_FCNTL(fds[0], F_GETFL);
+
+	if (!(flags & O_NONBLOCK))
+		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
+
+	/*
+	 * Get the size of the pipe.
+	 */
+	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
+
+	if (pipe_size != page_size * PAGE_NR)
+		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
+
+	/*
+	 * 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.
+	 *
+	 * Moreover, the size of a pipe is exprimed in page.
+	 * So the minimal size of a pipe is a page size, setting its size to 0
+	 * leads to a pipe whom size is 4096B.
+	 */
+	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);
+
+	/*
+	 * Create a buffer of page size.
+	 * We create it in stack because we do not care of its lifetime.
+	 */
+	buf = alloca(page_size);
+
+	SAFE_WRITE(1, fds[WRITE_SIDE], buf, page_size);
+
+	/*
+	 * This write should return -1 because pipe is already full.
+	 */
+	if (write(fds[WRITE_SIDE], buf, page_size) != -1)
+		tst_res(TFAIL | TERRNO, "write() succeeded and should not");
+
+	/*
+	 * Remove the O_NONBLOCK parameter for the pipe.
+	 * After this call write to the pipe will be blocking.
+	 */
+	SAFE_FCNTL(fds[0], F_SETFL, flags & ~O_NONBLOCK);
+
+	/*
+	 * Get again the flags of the pipe.
+	 */
+	flags = SAFE_FCNTL(fds[0], F_GETFL);
+
+	if (flags & O_NONBLOCK)
+		tst_res(TFAIL, "O_NONBLOCK flag must not be set.");
+
+	/*
+	 * Empty the set so no garbage value is in it.
+	 */
+	FD_ZERO(&set);
+
+	/*
+	 * Add the write side of the pipe to the set.
+	 */
+	FD_SET(fds[WRITE_SIDE], &set);
+
+	if (!FD_ISSET(fds[WRITE_SIDE], &set))
+		tst_res(TFAIL, "Pipe must be in the set.");
+
+	timeout.tv_sec = SECONDS;
+	timeout.tv_usec = MICROSECONDS;
+
+	/*
+	 * Since writes are now blocking we use select to check if the pipe is
+	 * available to receive write.
+	 * Wait SECONDS seconds and MICROSECONDS microseconds on write side of
+	 * the pipe.
+	 */
+	ret = select(1, NULL, &set, NULL, &timeout);
+
+	if (ret == -1)
+		tst_res(TFAIL | TERRNO, "select() failed");
+
+	/*
+	 * The pipe is still full so select should return after the waiting time
+	 * returning 0 because write side of the pipe is not available because
+	 * it is full.
+	 */
+	if (ret)
+		tst_res(TFAIL, "Pipe is not full.");
+	else
+		tst_res(TPASS, "Pipe is still full.");
+}
+
+/**
+ * Clean everything either if test is finished or if something failed.
+ */
+static void cleanup(void)
+{
+	for (int i = 0; i < 2; i++)
+		if (fds[i])
+			SAFE_CLOSE(fds[i]);
+}
+
+static struct tst_test test = {
+	.test_all = test_pipe2,
+	.setup = setup,
+	.cleanup = cleanup,
+};
\ No newline@end of file
-- 
2.20.1

[LTP] [PATCH] Increase pipe2() coverage.

[Yang Xu]

Hi laniel

Thanks for you patch, here are some small nits

Also, I think we can change a accurate subject, ie
"add new test for pipe2 with/without O_NONBLOCK mode"
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> A new test was added (pipe2_03.c), this test checks the following:
> 1. Create a pipe with O_NONBLOCK.
> 2. Check that this flag is set.
> 3. Check that pipe size is 65536B.
16 * page_size
> 4. Reduce pipe size to 4096B.
page_size
> 5. Write buffer bigger than page size and see that second write fails.
we should also check errno num
> 6. Set pipe's flags to default.
> 7. Check that pipe is still full with select.
IMO, we can move this function into child process, so we can check its 
status(it should be in s status when we write data into full pipe with 
block mode). And then we can kill it.
> ---
>   testcases/kernel/syscalls/pipe2/.gitignore |   1 +
>   testcases/kernel/syscalls/pipe2/pipe2_03.c | 215 +++++++++++++++++++++
>   2 files changed, 216 insertions(+)It miss runtest/syscalls file.
>   create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
> 
> 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..ea225fddc
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Iventifier: GPL-2.0
It should use SPDX-License-Identifier: GPL-2.0-or-later.
> +/* Copyright (c) Francis Laniel, 2020                                         */
> +/******************************************************************************/
> +/******************************************************************************/
> +/*                                                                            */
> +/* File:        pipe2_03.c                                                    */
> +/*                                                                            */
> +/* 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 or not.                    */
> +/*                                                                            */
> +/* Usage:  <for command-line>                                                 */
> +/* pipe2_03 [-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.                                */
> +/*
This is old style. I have mentioned that you may see pipe12.c for 
reference. In this case, option has been hanlded by ltp library, ie -i 
-t option. So we don't need this info.
 
     */
> +/* Total Tests: 1                                                             */
> +/*                                                                            */
> +/* Test Name:   pipe2_03                                                      */
> +/*                                                                            */
> +/* Author:      Francis Laniel                                                */
> +/*                                                                            */
> +/* History:     Created - Mar 28 2020 - Francis Laniel                        */
> +/******************************************************************************/
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <features.h>
> +#include <fcntl.h>
We should use lapi/fcntl.h to avoid undefined error(F_GETPIPE_SZ)  on 
centos6.
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <assert.h>
> +#include <sys/select.h>
> +
> +#include "tst_test.h"
> +
> +#define PAGE_NR 16
> +#define READ_SIDE 0
> +#define WRITE_SIDE 1
I guess we can drop READ_SIDE and WRITE_SIDE macro. We all know
fd[0] represent read side and fd[1] represent write side.
> +#define SECONDS 3
> +#define MICROSECONDS 0
> +
> +/**
> + * The two file descriptors of the pipe.
> + */
the comment is surplus.
> +static int fds[2];
> +
> +/**
> + * This variable will contain the system page size after setup.
> + */
here as well
> +static long page_size;
> +
> +/**
> + * Setup everything.
> + * This function will:
> + * 1. Create the pipe with O_NONBLOCK.
> + * 2. Get system page size with sysconf().
> + */
here as well
> +static void setup(void)
> +{
> +	/*
> +	 * Check that Linux version is greater than 2.6.35 to be able to use
> +	 * F_GETPIPE_SZ and F_SETPIPE_SZ.
> +	 */
> +	if (tst_kvercmp(2, 6, 35) < 0)
> +		tst_brk(TCONF, "This test can only run on kernels that are 2.6.35 and higher");
> +       
AFAIK, now linux kernel has reached to 5.7, 2.6.35 is so old that we 
don't need to compare this. Only ensuring this case can be compile is 
ok. ltp community has a discussion about minimal supported kernel and 
glibc version, more info see[1]

[1]https://github.com/linux-test-project/ltp/issues/657
> +	/*
> +	 * Create the pip with O_NONBLOCK.
> +	 */
> +	if (pipe2(fds, O_NONBLOCK))
> +		tst_brk(TBROK | TERRNO, "pipe2() failed");
I guess we can add SAFE_PIPE2 macro for this like SAFE_PIPE.
> +
> +	/*
> +	 * Get the system page size.
> +	 */
> +	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
> +}
> +
> +/**
> + * Test everything.
> + */
remove this comment.
> +static void test_pipe2(void)
> +{
> +	int ret;
> +
> +	long flags;
> +	long pipe_size;
> +
> +	char *buf;
> +
> +	fd_set set;
> +	struct timeval timeout;
> +
> +	/*
> +	 * Get the flags of the pipe.
> +	 */
remove...
> +	flags = SAFE_FCNTL(fds[0], F_GETFL);
> +
> +	if (!(flags & O_NONBLOCK))
> +		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
> +
> +	/*
> +	 * Get the size of the pipe.
> +	 */
remove
> +	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
> +
> +	if (pipe_size != page_size * PAGE_NR)
> +		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
> +
> +	/*
> +	 * 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.
> +	 *
> +	 * Moreover, the size of a pipe is exprimed in page.
> +	 * So the minimal size of a pipe is a page size, setting its size to 0
> +	 * leads to a pipe whom size is 4096B.
> +	 */Remove...
We all know if we set a size less than page size, it will round up to a 
page size not 4096b.
> +	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);
> +
> +	/*
> +	 * Create a buffer of page size.
> +	 * We create it in stack because we do not care of its lifetime.
> +	 */
Remove...
> +	buf = alloca(page_size);
Maybe we can use tst_alloc function. more info see[2]

https://githuber/doc/test-writing-g.com/linux-test-project/ltp/blob/mastuidelines.txt
> +
> +	SAFE_WRITE(1, fds[WRITE_SIDE], buf, page_size);
> +
> +	/*
> +	 * This write should return -1 because pipe is already full.
> +	 */
> +	if (write(fds[WRITE_SIDE], buf, page_size) != -1)
> +		tst_res(TFAIL | TERRNO, "write() succeeded and should not");
> +
> +	/*
> +	 * Remove the O_NONBLOCK parameter for the pipe.
> +	 * After this call write to the pipe will be blocking.
> +	 */
remove...
> +	SAFE_FCNTL(fds[0], F_SETFL, flags & ~O_NONBLOCK);
> +
> +	/*
> +	 * Get again the flags of the pipe.
> +	 */
remove...
> +	flags = SAFE_FCNTL(fds[0], F_GETFL);
> +
> +	if (flags & O_NONBLOCK)
> +		tst_res(TFAIL, "O_NONBLOCK flag must not be set.");
> +
> +	/*
> +	 * Empty the set so no garbage value is in it.
> +	 */
> +	FD_ZERO(&set);
> +
> +	/*
> +	 * Add the write side of the pipe to the set.
> +	 */
> +	FD_SET(fds[WRITE_SIDE], &set);
> +
> +	if (!FD_ISSET(fds[WRITE_SIDE], &set))
> +		tst_res(TFAIL, "Pipe must be in the set.");
> +
> +	timeout.tv_sec = SECONDS;
> +	timeout.tv_usec = MICROSECONDS;
> +
> +	/*
> +	 * Since writes are now blocking we use select to check if the pipe is
> +	 * available to receive write.
> +	 * Wait SECONDS seconds and MICROSECONDS microseconds on write side of
> +	 * the pipe.
> +	 */
> +	ret = select(1, NULL, &set, NULL, &timeout);
> +
> +	if (ret == -1)
> +		tst_res(TFAIL | TERRNO, "select() failed");
> +
> +	/*
> +	 * The pipe is still full so select should return after the waiting time
> +	 * returning 0 because write side of the pipe is not available because
> +	 * it is full.
> +	 */
> +	if (ret)
> +		tst_res(TFAIL, "Pipe is not full.");
> +	else
> +		tst_res(TPASS, "Pipe is still full.");
> +}
> +
> +/**
> + * Clean everything either if test is finished or if something failed.
> + */
> +static void cleanup(void)
> +{
> +	for (int i = 0; i < 2; i++)
> +		if (fds[i])
it should be if (fds[i] > 0)
> +			SAFE_CLOSE(fds[i]);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = test_pipe2,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};
> \ No newline at end of file
> 

[LTP] [PATCH] Increase pipe2() coverage.

[Francis Laniel]

Hi!


Thank you for the advice!

I just have a small question (see below).
> Le lundi 6 avril 2020, 07:57:02 CEST Yang Xu a ?crit :
> Hi laniel
> 
> Thanks for you patch, here are some small nits
> 
> Also, I think we can change a accurate subject, ie
> "add new test for pipe2 with/without O_NONBLOCK mode"
> 
> > From: Francis Laniel <laniel_francis@privacyrequired.com>
> > 
> > A new test was added (pipe2_03.c), this test checks the following:
> > 1. Create a pipe with O_NONBLOCK.
> > 2. Check that this flag is set.
> > 3. Check that pipe size is 65536B.
> 
> 16 * page_size
> 
> > 4. Reduce pipe size to 4096B.
> 
> page_size
> 
> > 5. Write buffer bigger than page size and see that second write fails.
> 
> we should also check errno num
> 
> > 6. Set pipe's flags to default.
> > 7. Check that pipe is still full with select.
> 
> IMO, we can move this function into child process, so we can check its
> status(it should be in s status when we write data into full pipe with
> block mode). And then we can kill it.

I did not understand what the child should execute?
If it writes it will block forever and so the father if it waits for its 
child.

> 
> > ---
> > 
> >   testcases/kernel/syscalls/pipe2/.gitignore |   1 +
> >   testcases/kernel/syscalls/pipe2/pipe2_03.c | 215 +++++++++++++++++++++
> >   2 files changed, 216 insertions(+)It miss runtest/syscalls file.
> >   create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
> > 
> > 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..ea225fddc
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> > @@ -0,0 +1,215 @@
> > +// SPDX-License-Iventifier: GPL-2.0
> 
> It should use SPDX-License-Identifier: GPL-2.0-or-later.
> 
> > +/* Copyright (c) Francis Laniel, 2020                                    
> >     */
> > +/***********************************************************************
> > *******/
> > +/***********************************************************************
> > *******/ +/*                                                              
> >              */ +/* File:        pipe2_03.c                              
> >                      */ +/*                                              
> >                              */ +/* 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 or not.                    */ +/*     
> >                                                                       */
> > +/* Usage:  <for command-line>                                           
> >      */ +/* pipe2_03 [-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.                                */
> > +/*
> 
> This is old style. I have mentioned that you may see pipe12.c for
> reference. In this case, option has been hanlded by ltp library, ie -i
> -t option. So we don't need this info.
> 
>      */
> 
> > +/* Total Tests: 1                                                        
> >     */ +/*                                                               
> >             */ +/* Test Name:   pipe2_03                                 
> >                     */ +/*                                               
> >                             */ +/* Author:      Francis Laniel           
> >                                     */ +/*                               
> >                                             */ +/* History:     Created -
> > Mar 28 2020 - Francis Laniel                        */
> > +/***********************************************************************
> > *******/ +#define _GNU_SOURCE
> > +#include <stdlib.h>
> > +#include <features.h>
> > +#include <fcntl.h>
> 
> We should use lapi/fcntl.h to avoid undefined error(F_GETPIPE_SZ)  on
> centos6.
> 
> > +#include <unistd.h>
> > +#include <stdio.h>
> > +#include <assert.h>
> > +#include <sys/select.h>
> > +
> > +#include "tst_test.h"
> > +
> > +#define PAGE_NR 16
> > +#define READ_SIDE 0
> > +#define WRITE_SIDE 1
> 
> I guess we can drop READ_SIDE and WRITE_SIDE macro. We all know
> fd[0] represent read side and fd[1] represent write side.
> 
> > +#define SECONDS 3
> > +#define MICROSECONDS 0
> > +
> > +/**
> > + * The two file descriptors of the pipe.
> > + */
> 
> the comment is surplus.
> 
> > +static int fds[2];
> > +
> > +/**
> > + * This variable will contain the system page size after setup.
> > + */
> 
> here as well
> 
> > +static long page_size;
> > +
> > +/**
> > + * Setup everything.
> > + * This function will:
> > + * 1. Create the pipe with O_NONBLOCK.
> > + * 2. Get system page size with sysconf().
> > + */
> 
> here as well
> 
> > +static void setup(void)
> > +{
> > +	/*
> > +	 * Check that Linux version is greater than 2.6.35 to be able to use
> > +	 * F_GETPIPE_SZ and F_SETPIPE_SZ.
> > +	 */
> > +	if (tst_kvercmp(2, 6, 35) < 0)
> > +		tst_brk(TCONF, "This test can only run on kernels that are 2.6.35 and
> > higher"); +
> 
> AFAIK, now linux kernel has reached to 5.7, 2.6.35 is so old that we
> don't need to compare this. Only ensuring this case can be compile is
> ok. ltp community has a discussion about minimal supported kernel and
> glibc version, more info see[1]
> 
> [1]https://github.com/linux-test-project/ltp/issues/657
> 
> > +	/*
> > +	 * Create the pip with O_NONBLOCK.
> > +	 */
> > +	if (pipe2(fds, O_NONBLOCK))
> > +		tst_brk(TBROK | TERRNO, "pipe2() failed");
> 
> I guess we can add SAFE_PIPE2 macro for this like SAFE_PIPE.
> 
> > +
> > +	/*
> > +	 * Get the system page size.
> > +	 */
> > +	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
> > +}
> > +
> > +/**
> > + * Test everything.
> > + */
> 
> remove this comment.
> 
> > +static void test_pipe2(void)
> > +{
> > +	int ret;
> > +
> > +	long flags;
> > +	long pipe_size;
> > +
> > +	char *buf;
> > +
> > +	fd_set set;
> > +	struct timeval timeout;
> > +
> > +	/*
> > +	 * Get the flags of the pipe.
> > +	 */
> 
> remove...
> 
> > +	flags = SAFE_FCNTL(fds[0], F_GETFL);
> > +
> > +	if (!(flags & O_NONBLOCK))
> > +		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
> > +
> > +	/*
> > +	 * Get the size of the pipe.
> > +	 */
> 
> remove
> 
> > +	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
> > +
> > +	if (pipe_size != page_size * PAGE_NR)
> > +		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
> > +
> > +	/*
> > +	 * 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.
> > +	 *
> > +	 * Moreover, the size of a pipe is exprimed in page.
> > +	 * So the minimal size of a pipe is a page size, setting its size to 0
> > +	 * leads to a pipe whom size is 4096B.
> > +	 */Remove...
> 
> We all know if we set a size less than page size, it will round up to a
> page size not 4096b.
> 
> > +	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);
> > +
> > +	/*
> > +	 * Create a buffer of page size.
> > +	 * We create it in stack because we do not care of its lifetime.
> > +	 */
> 
> Remove...
> 
> > +	buf = alloca(page_size);
> 
> Maybe we can use tst_alloc function. more info see[2]
> 
> https://githuber/doc/test-writing-g.com/linux-test-project/ltp/blob/mastuide
> lines.txt
> > +
> > +	SAFE_WRITE(1, fds[WRITE_SIDE], buf, page_size);
> > +
> > +	/*
> > +	 * This write should return -1 because pipe is already full.
> > +	 */
> > +	if (write(fds[WRITE_SIDE], buf, page_size) != -1)
> > +		tst_res(TFAIL | TERRNO, "write() succeeded and should not");
> > +
> > +	/*
> > +	 * Remove the O_NONBLOCK parameter for the pipe.
> > +	 * After this call write to the pipe will be blocking.
> > +	 */
> 
> remove...
> 
> > +	SAFE_FCNTL(fds[0], F_SETFL, flags & ~O_NONBLOCK);
> > +
> > +	/*
> > +	 * Get again the flags of the pipe.
> > +	 */
> 
> remove...
> 
> > +	flags = SAFE_FCNTL(fds[0], F_GETFL);
> > +
> > +	if (flags & O_NONBLOCK)
> > +		tst_res(TFAIL, "O_NONBLOCK flag must not be set.");
> > +
> > +	/*
> > +	 * Empty the set so no garbage value is in it.
> > +	 */
> > +	FD_ZERO(&set);
> > +
> > +	/*
> > +	 * Add the write side of the pipe to the set.
> > +	 */
> > +	FD_SET(fds[WRITE_SIDE], &set);
> > +
> > +	if (!FD_ISSET(fds[WRITE_SIDE], &set))
> > +		tst_res(TFAIL, "Pipe must be in the set.");
> > +
> > +	timeout.tv_sec = SECONDS;
> > +	timeout.tv_usec = MICROSECONDS;
> > +
> > +	/*
> > +	 * Since writes are now blocking we use select to check if the pipe is
> > +	 * available to receive write.
> > +	 * Wait SECONDS seconds and MICROSECONDS microseconds on write side of
> > +	 * the pipe.
> > +	 */
> > +	ret = select(1, NULL, &set, NULL, &timeout);
> > +
> > +	if (ret == -1)
> > +		tst_res(TFAIL | TERRNO, "select() failed");
> > +
> > +	/*
> > +	 * The pipe is still full so select should return after the waiting time
> > +	 * returning 0 because write side of the pipe is not available because
> > +	 * it is full.
> > +	 */
> > +	if (ret)
> > +		tst_res(TFAIL, "Pipe is not full.");
> > +	else
> > +		tst_res(TPASS, "Pipe is still full.");
> > +}
> > +
> > +/**
> > + * Clean everything either if test is finished or if something failed.
> > + */
> > +static void cleanup(void)
> > +{
> > +	for (int i = 0; i < 2; i++)
> > +		if (fds[i])
> 
> it should be if (fds[i] > 0)
> 
> > +			SAFE_CLOSE(fds[i]);
> > +}
> > +
> > +static struct tst_test test = {
> > +	.test_all = test_pipe2,
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +};
> > \ No newline at end of file

[LTP] [PATCH] Increase pipe2() coverage.

[Yang Xu]

Hi Francis

> Hi!
> 
> 
> Thank you for the advice!
> 
> I just have a small question (see below).
>> Le lundi 6 avril 2020, 07:57:02 CEST Yang Xu a ?crit :
>> Hi laniel
>>
>> Thanks for you patch, here are some small nits
>>
>> Also, I think we can change a accurate subject, ie
>> "add new test for pipe2 with/without O_NONBLOCK mode"
>>
>>> From: Francis Laniel <laniel_francis@privacyrequired.com>
>>>
>>> A new test was added (pipe2_03.c), this test checks the following:
>>> 1. Create a pipe with O_NONBLOCK.
>>> 2. Check that this flag is set.
>>> 3. Check that pipe size is 65536B.
>>
>> 16 * page_size
>>
>>> 4. Reduce pipe size to 4096B.
>>
>> page_size
>>
>>> 5. Write buffer bigger than page size and see that second write fails.
>>
>> we should also check errno num
>>
>>> 6. Set pipe's flags to default.
>>> 7. Check that pipe is still full with select.
>>
>> IMO, we can move this function into child process, so we can check its
>> status(it should be in s status when we write data into full pipe with
>> block mode). And then we can kill it.
> 
> I did not understand what the child should execute?
> If it writes it will block forever and so the father if it waits for its
> child.Parent will kill its child and wait. We have TST_PROCESS_STATE_WAIT to 
check whether child is in block stat.

We have a test case vmsplice04, it is similar to this case, I think you 
can refer to it[1].

[1]https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/vmsplice/vmsplice04.c
> 
>>
>>> ---
>>>
>>>    testcases/kernel/syscalls/pipe2/.gitignore |   1 +
>>>    testcases/kernel/syscalls/pipe2/pipe2_03.c | 215 +++++++++++++++++++++
>>>    2 files changed, 216 insertions(+)It miss runtest/syscalls file.
>>>    create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
>>>
>>> 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..ea225fddc
>>> --- /dev/null
>>> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
>>> @@ -0,0 +1,215 @@
>>> +// SPDX-License-Iventifier: GPL-2.0
>>
>> It should use SPDX-License-Identifier: GPL-2.0-or-later.
>>
>>> +/* Copyright (c) Francis Laniel, 2020
>>>      */
>>> +/***********************************************************************
>>> *******/
>>> +/***********************************************************************
>>> *******/ +/*
>>>               */ +/* File:        pipe2_03.c
>>>                       */ +/*
>>>                               */ +/* 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 or not.                    */ +/*
>>>                                                                        */
>>> +/* Usage:  <for command-line>
>>>       */ +/* pipe2_03 [-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.                                */
>>> +/*
>>
>> This is old style. I have mentioned that you may see pipe12.c for
>> reference. In this case, option has been hanlded by ltp library, ie -i
>> -t option. So we don't need this info.
>>
>>       */
>>
>>> +/* Total Tests: 1
>>>      */ +/*
>>>              */ +/* Test Name:   pipe2_03
>>>                      */ +/*
>>>                              */ +/* Author:      Francis Laniel
>>>                                      */ +/*
>>>                                              */ +/* History:     Created -
>>> Mar 28 2020 - Francis Laniel                        */
>>> +/***********************************************************************
>>> *******/ +#define _GNU_SOURCE
>>> +#include <stdlib.h>
>>> +#include <features.h>
>>> +#include <fcntl.h>
>>
>> We should use lapi/fcntl.h to avoid undefined error(F_GETPIPE_SZ)  on
>> centos6.
>>
>>> +#include <unistd.h>
>>> +#include <stdio.h>
>>> +#include <assert.h>
>>> +#include <sys/select.h>
>>> +
>>> +#include "tst_test.h"
>>> +
>>> +#define PAGE_NR 16
>>> +#define READ_SIDE 0
>>> +#define WRITE_SIDE 1
>>
>> I guess we can drop READ_SIDE and WRITE_SIDE macro. We all know
>> fd[0] represent read side and fd[1] represent write side.
>>
>>> +#define SECONDS 3
>>> +#define MICROSECONDS 0
>>> +
>>> +/**
>>> + * The two file descriptors of the pipe.
>>> + */
>>
>> the comment is surplus.
>>
>>> +static int fds[2];
>>> +
>>> +/**
>>> + * This variable will contain the system page size after setup.
>>> + */
>>
>> here as well
>>
>>> +static long page_size;
>>> +
>>> +/**
>>> + * Setup everything.
>>> + * This function will:
>>> + * 1. Create the pipe with O_NONBLOCK.
>>> + * 2. Get system page size with sysconf().
>>> + */
>>
>> here as well
>>
>>> +static void setup(void)
>>> +{
>>> +	/*
>>> +	 * Check that Linux version is greater than 2.6.35 to be able to use
>>> +	 * F_GETPIPE_SZ and F_SETPIPE_SZ.
>>> +	 */
>>> +	if (tst_kvercmp(2, 6, 35) < 0)
>>> +		tst_brk(TCONF, "This test can only run on kernels that are 2.6.35 and
>>> higher"); +
>>
>> AFAIK, now linux kernel has reached to 5.7, 2.6.35 is so old that we
>> don't need to compare this. Only ensuring this case can be compile is
>> ok. ltp community has a discussion about minimal supported kernel and
>> glibc version, more info see[1]
>>
>> [1]https://github.com/linux-test-project/ltp/issues/657
>>
>>> +	/*
>>> +	 * Create the pip with O_NONBLOCK.
>>> +	 */
>>> +	if (pipe2(fds, O_NONBLOCK))
>>> +		tst_brk(TBROK | TERRNO, "pipe2() failed");
>>
>> I guess we can add SAFE_PIPE2 macro for this like SAFE_PIPE.
>>
>>> +
>>> +	/*
>>> +	 * Get the system page size.
>>> +	 */
>>> +	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
>>> +}
>>> +
>>> +/**
>>> + * Test everything.
>>> + */
>>
>> remove this comment.
>>
>>> +static void test_pipe2(void)
>>> +{
>>> +	int ret;
>>> +
>>> +	long flags;
>>> +	long pipe_size;
>>> +
>>> +	char *buf;
>>> +
>>> +	fd_set set;
>>> +	struct timeval timeout;
>>> +
>>> +	/*
>>> +	 * Get the flags of the pipe.
>>> +	 */
>>
>> remove...
>>
>>> +	flags = SAFE_FCNTL(fds[0], F_GETFL);
>>> +
>>> +	if (!(flags & O_NONBLOCK))
>>> +		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
>>> +
>>> +	/*
>>> +	 * Get the size of the pipe.
>>> +	 */
>>
>> remove
>>
>>> +	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
>>> +
>>> +	if (pipe_size != page_size * PAGE_NR)
>>> +		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
>>> +
>>> +	/*
>>> +	 * 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.
>>> +	 *
>>> +	 * Moreover, the size of a pipe is exprimed in page.
>>> +	 * So the minimal size of a pipe is a page size, setting its size to 0
>>> +	 * leads to a pipe whom size is 4096B.
>>> +	 */Remove...
>>
>> We all know if we set a size less than page size, it will round up to a
>> page size not 4096b.
>>
>>> +	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);
>>> +
>>> +	/*
>>> +	 * Create a buffer of page size.
>>> +	 * We create it in stack because we do not care of its lifetime.
>>> +	 */
>>
>> Remove...
>>
>>> +	buf = alloca(page_size);
>>
>> Maybe we can use tst_alloc function. more info see[2]
>>
>> https://githuber/doc/test-writing-g.com/linux-test-project/ltp/blob/mastuide
>> lines.txt
>>> +
>>> +	SAFE_WRITE(1, fds[WRITE_SIDE], buf, page_size);
>>> +
>>> +	/*
>>> +	 * This write should return -1 because pipe is already full.
>>> +	 */
>>> +	if (write(fds[WRITE_SIDE], buf, page_size) != -1)
>>> +		tst_res(TFAIL | TERRNO, "write() succeeded and should not");
>>> +
>>> +	/*
>>> +	 * Remove the O_NONBLOCK parameter for the pipe.
>>> +	 * After this call write to the pipe will be blocking.
>>> +	 */
>>
>> remove...
>>
>>> +	SAFE_FCNTL(fds[0], F_SETFL, flags & ~O_NONBLOCK);
>>> +
>>> +	/*
>>> +	 * Get again the flags of the pipe.
>>> +	 */
>>
>> remove...
>>
>>> +	flags = SAFE_FCNTL(fds[0], F_GETFL);
>>> +
>>> +	if (flags & O_NONBLOCK)
>>> +		tst_res(TFAIL, "O_NONBLOCK flag must not be set.")
>>> +
>>> +	/*
>>> +	 * Empty the set so no garbage value is in it.
>>> +	 */
>>> +	FD_ZERO(&set);
>>> +
>>> +	/*
>>> +	 * Add the write side of the pipe to the set.
>>> +	 */
>>> +	FD_SET(fds[WRITE_SIDE], &set);
>>> +
>>> +	if (!FD_ISSET(fds[WRITE_SIDE], &set))
>>> +		tst_res(TFAIL, "Pipe must be in the set.");
>>> +
>>> +	timeout.tv_sec = SECONDS;
>>> +	timeout.tv_usec = MICROSECONDS;
>>> +
>>> +	/*
>>> +	 * Since writes are now blocking we use select to check if the pipe is
>>> +	 * available to receive write.
>>> +	 * Wait SECONDS seconds and MICROSECONDS microseconds on write side of
>>> +	 * the pipe.
>>> +	 */
>>> +	ret = select(1, NULL, &set, NULL, &timeout);
>>> +
>>> +	if (ret == -1)
>>> +		tst_res(TFAIL | TERRNO, "select() failed");
>>> +
>>> +	/*
>>> +	 * The pipe is still full so select should return after the waiting time
>>> +	 * returning 0 because write side of the pipe is not available because
>>> +	 * it is full.
>>> +	 */
>>> +	if (ret)
>>> +		tst_res(TFAIL, "Pipe is not full.");
>>> +	else
>>> +		tst_res(TPASS, "Pipe is still full.");
>>> +}
>>> +
>>> +/**
>>> + * Clean everything either if test is finished or if something failed.
>>> + */
>>> +static void cleanup(void)
>>> +{
>>> +	for (int i = 0; i < 2; i++)
>>> +		if (fds[i])
>>
>> it should be if (fds[i] > 0)
>>
>>> +			SAFE_CLOSE(fds[i]);
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +	.test_all = test_pipe2,
>>> +	.setup = setup,
>>> +	.cleanup = cleanup,
>>> +};
>>> \ No newline at end of file
> 
> 
> 
> 
> 

[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.

[LTP] [PATCH v2 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/safe_macros_fn.h  |  3 +++
 include/tst_safe_macros.h |  3 +++
 lib/safe_macros.c         | 15 +++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
index 3df952811..ec2d34ae3 100644
--- a/include/safe_macros_fn.h
+++ b/include/safe_macros_fn.h
@@ -67,6 +67,9 @@ int safe_open(const char *file, const int lineno,
 int safe_pipe(const char *file, const int lineno,
               void (*cleanup_fn)(void), int fildes[2]);
 
+int safe_pipe2(const char *file, const int lineno, void (*cleanup_fn) (void),
+               int fildes[2], int flags);
+
 ssize_t safe_read(const char *file, const int lineno,
                   void (*cleanup_fn)(void), char len_strict, int fildes,
                   void *buf, size_t nbyte);
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index d95d26219..1738d3cc6 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -87,6 +87,9 @@ static inline int safe_dup(const char *file, const int lineno,
 #define SAFE_PIPE(fildes) \
 	safe_pipe(__FILE__, __LINE__, NULL, (fildes))
 
+#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 v2 1/2] Add SAFE_PIPE2.

[Yang Xu]

Hi Laniel

Usually, we should add safe macro for new and old api.
We have old/safe_macros.h.

But ltp only has three places to use pipe2(pipe2_01,02,03), and
pipe2_01,02 use ltp_syscall to call pipe2, so I think it is ok.

only a small nit:
move safe_pipe2() declaration to tst_safe_macros.h. We don't need to add 
it into safe_macros_fn.h. ie commit f59fa0de ("Add SAFE_PTRACE() to LTP 
library").

Reviewed-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Best Regards
Yang Xu
> From: Francis Laniel <laniel_francis@privacyrequired.com>
> 
> This macro is the equivalent of SAFE_PIPE for pipe2 system call.
> ---
>   include/safe_macros_fn.h  |  3 +++
>   include/tst_safe_macros.h |  3 +++
>   lib/safe_macros.c         | 15 +++++++++++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h
> index 3df952811..ec2d34ae3 100644
> --- a/include/safe_macros_fn.h
> +++ b/include/safe_macros_fn.h
> @@ -67,6 +67,9 @@ int safe_open(const char *file, const int lineno,
>   int safe_pipe(const char *file, const int lineno,
>                 void (*cleanup_fn)(void), int fildes[2]);
>   
> +int safe_pipe2(const char *file, const int lineno, void (*cleanup_fn) (void),
> +               int fildes[2], int flags);
> +
>   ssize_t safe_read(const char *file, const int lineno,
>                     void (*cleanup_fn)(void), char len_strict, int fildes,
>                     void *buf, size_t nbyte);
> diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
> index d95d26219..1738d3cc6 100644
> --- a/include/tst_safe_macros.h
> +++ b/include/tst_safe_macros.h
> @@ -87,6 +87,9 @@ static inline int safe_dup(const char *file, const int lineno,
>   #define SAFE_PIPE(fildes) \
>   	safe_pipe(__FILE__, __LINE__, NULL, (fildes))
>   
> +#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)
>   {
> 

[LTP] [PATCH v2 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.
---
 testcases/kernel/syscalls/pipe2/.gitignore |   1 +
 testcases/kernel/syscalls/pipe2/pipe2_03.c | 128 +++++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c

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..c2b182e02
--- /dev/null
+++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
@@ -0,0 +1,128 @@
+// 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"
+
+#define PAGE_NR 16
+#define SECONDS 3
+#define MICROSECONDS 0
+
+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[0], F_GETFL);
+
+	if (!(flags & O_NONBLOCK))
+		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
+
+	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
+
+	if (pipe_size != page_size * PAGE_NR)
+		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
+
+	/*
+	 * 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 because pipe is already full.
+	 */
+	if (write(fds[1], buf, page_size) != -1)
+		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);
+}
+
+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] [PATCH v2 2/2] Add new test for pipe2 with/without O_NONBLOCK mode.

[Yang Xu]

Hi laniel

> 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.
> ---
>   testcases/kernel/syscalls/pipe2/.gitignore |   1 +
>   testcases/kernel/syscalls/pipe2/pipe2_03.c | 128 +++++++++++++++++++++
We should add pipe2_03 into runtest/syscalls file, so we can use runltp 
command to run this case when make install.  Also see Contribution 
Checklist in ltp/doc/test-writing-guidelines.txt

4. Test Contribution Checklist
------------------------------

1. Test compiles and runs fine (check with -i 10 too)
2. Checkpatch does not report any errors
3. The runtest entires are in place
4. Test files are added into corresponding .gitignore files
5. Patches apply over the latest git

>   2 files changed, 129 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/pipe2/pipe2_03.c
> 
> 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..c2b182e02
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pipe2/pipe2_03.c
> @@ -0,0 +1,128 @@
> +// 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"
> +
> +#define PAGE_NR 16
> +#define SECONDS 3
> +#define MICROSECONDS 0
The two variables is useless.
> +
> +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[0], F_GETFL);
> +
> +	if (!(flags & O_NONBLOCK))
> +		tst_res(TFAIL, "O_NONBLOCK flag must be set.");
> +
> +	pipe_size = SAFE_FCNTL(fds[0], F_GETPIPE_SZ);
> +
> +	if (pipe_size != page_size * PAGE_NR)
> +		tst_res(TFAIL, "Default pipe page is 16 * 4096 = 65536B.");
I guess we can remove this check. We can not ensure kernel will keep 
default pipe size is equal to 16 pagesize in future. We only ensure this 
pipe is full is ok. We can fill this pipe in setup parse.
> +
> +	/*
> +	 * 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 because pipe is already full.
> +	 */
> +	if (write(fds[1], buf, page_size) != -1)
> +		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);
> +}
> +
> +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
>