Increase pipe2() coverage.

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

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
>

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

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

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.