Re: [LTP] [PATCH 1/2] syscalls/preadv: Make make check happy

[Petr Vorel <pvorel@suse.cz>]

Hi Xiao Yang,

> From: Xiao Yang <yangx.jy@fujitsu.com>

> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
>  testcases/kernel/syscalls/preadv/preadv.h   | 19 ++------
>  testcases/kernel/syscalls/preadv/preadv01.c | 26 +++++------
>  testcases/kernel/syscalls/preadv/preadv02.c | 50 ++++++++++-----------
>  3 files changed, 42 insertions(+), 53 deletions(-)

> diff --git a/testcases/kernel/syscalls/preadv/preadv.h b/testcases/kernel/syscalls/preadv/preadv.h
> index 73466a9aa..c3e9e5f19 100644
> --- a/testcases/kernel/syscalls/preadv/preadv.h
> +++ b/testcases/kernel/syscalls/preadv/preadv.h
> @@ -1,18 +1,7 @@
> -/*
> -* Copyright (c) 2015 Fujitsu Ltd.
> -* Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> -*
> -* This program is free software; you can redistribute it and/or modify it
> -* under the terms of version 2 of the GNU General Public License as
> -* published by the Free Software Foundation.
> -*
> -* This program is distributed in the hope that it would be useful, but
> -* WITHOUT ANY WARRANTY; without even the implied warranty of
> -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> -*
> -* You should have received a copy of the GNU General Public License
> -* alone with this program.
> -*/
> +/* SPDX-License-Identifier: GPL-2.0-or-later
So you relicense from GPL v2 only to GPL v2+. You're the author, so you can do
it (normally we shouldn't relicense). But maybe note that at the commit message?
Thanks!

> + * Copyright (c) 2015 Fujitsu Ltd.
nit: maybe add LTP copyright?
* Copyright (c) Linux Test Project, 2016-2017

> + * Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> + */

>  #ifndef PREADV_H
>  #define PREADV_H
> diff --git a/testcases/kernel/syscalls/preadv/preadv01.c b/testcases/kernel/syscalls/preadv/preadv01.c
> index 62f9296f2..aba4748a5 100644
> --- a/testcases/kernel/syscalls/preadv/preadv01.c
> +++ b/testcases/kernel/syscalls/preadv/preadv01.c
> @@ -1,17 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> -* Copyright (c) 2015 Fujitsu Ltd.
> -* Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> -*/
> + * Copyright (c) 2015 Fujitsu Ltd.
nit: maybe add LTP copyright?
* Copyright (c) Linux Test Project, 2016-2022
> + * Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> + */

>  /*
> -* Test Name: preadv01
> -*
> -* Test Description:
> -* Testcase to check the basic functionality of the preadv(2).
> -* Preadv(2) should succeed to read the expected content of data
> -* and after reading the file, the file offset is not changed.
> -*/
> + * Test Name: preadv01

Could you please convert to the docparse? Something like this:

/*\
 * [Description]

 * Testcase to check the basic functionality of the preadv(2).

 * preadv(2) should succeed to read the expected content of data
 * and after reading the file, the file offset is not changed.
 */

> + *
> + * Test Description:
> + * Testcase to check the basic functionality of the preadv(2).
> + * Preadv(2) should succeed to read the expected content of data
> + * and after reading the file, the file offset is not changed.
> + */

>  #define _GNU_SOURCE

> @@ -38,7 +38,7 @@ static struct tcase {
>  	{1, CHUNK*3/2, CHUNK/2, 'b'}
>  };

> -void verify_preadv(unsigned int n)
> +static void verify_preadv(unsigned int n)
>  {
>  	int i;
>  	char *vec;
> @@ -81,7 +81,7 @@ void verify_preadv(unsigned int n)
>  		 "with content '%c' expectedly", tc->size, tc->content);
>  }

> -void setup(void)
> +static void setup(void)
>  {
>  	char buf[CHUNK];

> @@ -94,7 +94,7 @@ void setup(void)
>  	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, sizeof(buf));
>  }

> -void cleanup(void)
> +static void cleanup(void)
>  {
>  	if (fd > 0)
>  		SAFE_CLOSE(fd);
> diff --git a/testcases/kernel/syscalls/preadv/preadv02.c b/testcases/kernel/syscalls/preadv/preadv02.c
> index 500059e42..65d4795d6 100644
> --- a/testcases/kernel/syscalls/preadv/preadv02.c
> +++ b/testcases/kernel/syscalls/preadv/preadv02.c
> @@ -1,32 +1,32 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> -* Copyright (c) 2015-2016 Fujitsu Ltd.
> -* Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> -*/
> + * Copyright (c) 2015-2016 Fujitsu Ltd.
> + * Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
> + */

>  /*
> + * Test Name: preadv02
nit: remove this ^ (useless)

Again, add docparse + simplify docs a bit:

/*\
 * [Description]
 *
 * Test basic error handling of the preadv(2) syscall.
 *
 * 1) preadv(2) fails if iov_len is invalid (EINVAL)
 * 2) preadv(2) fails if the vector count iovcnt is less than zero (EINVAL).
 * 3) preadv(2) fails if offset is negative (EINVAL).
 * 4) preadv(2) fails when attempts to read into a invalid address (EFAULT).
 * 5) preadv(2) fails if file descriptor is invalid (EBADF).
 * 6) preadv(2) fails if file descriptor is not open for reading (EBADF).
 * 7) preadv(2) fails when fd refers to a directory (EISDIR).
 * 8) preadv(2) fails if fd is associated with a pipe (ESPIPE).
 */

With this, you can add my:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> + *
> + * Description:
> + * 1) preadv(2) fails if iov_len is invalid.
> + * 2) preadv(2) fails if the vector count iovcnt is less than zero.
> + * 3) preadv(2) fails if offset is negative.
> + * 4) preadv(2) fails when attempts to read into a invalid address.
> + * 5) preadv(2) fails if file descriptor is invalid.
> + * 6) preadv(2) fails if file descriptor is not open for reading.
> + * 7) preadv(2) fails when fd refers to a directory.
> + * 8) preadv(2) fails if fd is associated with a pipe.
> + *
> + * Expected Result:
> + * 1) preadv(2) should return -1 and set errno to EINVAL.
> + * 2) preadv(2) should return -1 and set errno to EINVAL.
> + * 3) preadv(2) should return -1 and set errno to EINVAL.
> + * 4) preadv(2) should return -1 and set errno to EFAULT.
> + * 5) preadv(2) should return -1 and set errno to EBADF.
> + * 6) preadv(2) should return -1 and set errno to EBADF.
> + * 7) preadv(2) should return -1 and set errno to EISDIR.
> + * 8) preadv(2) should return -1 and set errno to ESPIPE.
> + */

>  #define _GNU_SOURCE

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp