diff mbox series

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

Message ID 20240113144940.133746-1-ice_yangxiao@163.com
State Changes Requested
Headers show
Series [1/2] syscalls/preadv: Make make check happy | expand

Commit Message

Xiao Yang Jan. 13, 2024, 2:49 p.m. UTC
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(-)

Comments

Petr Vorel Jan. 17, 2024, 9:37 a.m. UTC | #1
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
Xiao Yang Jan. 21, 2024, 12:32 p.m. UTC | #2
Hi Petr

Thanks a lot for your detailed comment.
I have sent the v2 patch set as you suggested.

Best Regards,
Xiao Yang

On 2024/1/17 17:37, Petr Vorel wrote:
> 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
diff mbox series

Patch

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
+ * Copyright (c) 2015 Fujitsu Ltd.
+ * 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.
+ * 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
+ *
+ * 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
-*
-* 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.
-*/
+ * Test Name: preadv02
+ *
+ * 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