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

[Xiao Yang]

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