[LTP] [PATCH] syscalls/write06: Add new test

[daisl.fnst@fujitsu.com]

Hi Cyril

在 2021/12/7 21:51, Cyril Hrubis 写道:
> Hi!
>> Signed-off-by: Dai Shili <daisl.fnst@fujitsu.com>
>> ---
>>   runtest/syscalls                           |  1 +
>>   testcases/kernel/syscalls/write/.gitignore |  1 +
>>   testcases/kernel/syscalls/write/write06.c  | 94 ++++++++++++++++++++++++++++++
>>   3 files changed, 96 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/write/write06.c
>>
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index bcf3d56..32fcda4 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1699,6 +1699,7 @@ write02 write02
>>   write03 write03
>>   write04 write04
>>   write05 write05
>> +write06 write06
>>   
>>   writev01 writev01
>>   writev02 writev02
>> diff --git a/testcases/kernel/syscalls/write/.gitignore b/testcases/kernel/syscalls/write/.gitignore
>> index 7f36194..8529aae 100644
>> --- a/testcases/kernel/syscalls/write/.gitignore
>> +++ b/testcases/kernel/syscalls/write/.gitignore
>> @@ -3,3 +3,4 @@
>>   /write03
>>   /write04
>>   /write05
>> +/write06
>> diff --git a/testcases/kernel/syscalls/write/write06.c b/testcases/kernel/syscalls/write/write06.c
>> new file mode 100644
>> index 0000000..c62a266
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/write/write06.c
>> @@ -0,0 +1,94 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>> + * Author: Dai Shili <daisl.fnst@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Test the write() system call with O_APPEND.
>> + *
>> + * Writing 2k data to the file, close it and reopen it with O_APPEND.
>> + * Verify that the file size is 3k and offset is moved to the end of the file.
>> + */
>> +
>> +#include <stdlib.h>
>> +#include <inttypes.h>
>> +#include "tst_test.h"
>> +#include "tst_safe_prw.h"
>> +
>> +#define K1              1024
>> +#define K2              (K1 * 2)
>> +#define K3              (K1 * 3)
>> +#define DATA_FILE       "write06_file"
>> +
>> +static int fd = -1;
>> +static char *write_buf[2];
>> +
>> +static void l_seek(int fdesc, off_t offset, int whence, off_t checkoff)
>> +{
>> +	off_t offloc;
>> +
>> +	offloc = SAFE_LSEEK(fdesc, offset, whence);
>> +	if (offloc != checkoff) {
>> +		tst_res(TFAIL, "%" PRId64 " = lseek(%d, %" PRId64 ", %d) != %" PRId64,
>> +			(int64_t)offloc, fdesc, (int64_t)offset, whence, (int64_t)checkoff);
>> +	}
>> +}
>> +
>> +static void verify_write(void)
>> +{
>> +	int fail = 0;
>> +	struct stat statbuf;
>> +
>> +	fd = SAFE_OPEN(DATA_FILE, O_RDWR | O_CREAT | O_TRUNC, 0666);
>> +	SAFE_WRITE(1, fd, write_buf[0], K2);
>> +	SAFE_CLOSE(fd);
>> +
>> +	fd = SAFE_OPEN(DATA_FILE, O_RDWR | O_APPEND, 0666);
>                                                       ^
> 						     No need to pass
> 						     mode without
> 						     O_CREAT
Got it.
>> +	SAFE_FSTAT(fd, &statbuf);
>> +	if (statbuf.st_size != K2) {
>> +		fail++;
>> +		tst_res(TFAIL, "file size is %ld != K2", statbuf.st_size);
>> +	}
>> +
>> +	l_seek(fd, K1, SEEK_SET, K1);
> Why do we do the seek here?
>
> Wouldn't it make much more sense just to write the write_buf[1] then
> check that the st_size is K3?
>
> That way we would check that O_APPEND opened file has correct position
> associated with the file descriptor.

I want to verify that the offset will move to the end of the file.

So I put the offset in the middle of the file before write.

>> +	SAFE_WRITE(1, fd, write_buf[1], K1);
>> +	l_seek(fd, 0, SEEK_CUR, K3);
> And here as well, why the seek? This is actually the place that
> increases the size not the write() as it should have been.
Do seek to check the offset is moved to the end of the file.
>> +	SAFE_FSTAT(fd, &statbuf);
>> +	if (statbuf.st_size != K3) {
>> +		fail++;
>> +		tst_res(TFAIL, "file size is %ld != K3", statbuf.st_size);
>> +	}
> Really the whole test should do:
>
> 	open(FILE, O_RDWR | O_CREAT | O_TRUNC, 0666)
> 	write()
> 	close()
>
> 	open(FILE, O_RDWR | O_APPEND)
> 	check size
> 	write()
> 	check size
> 	close()
>
> 	open(FILE, O_RDONLY);
> 	read()
> 	verify data
> 	close()
>
I'm so sorry, my commit msg is not clear.

What I want to test is the following description of open(2) man-pages:

O_APPEND
       The file is opened in append mode.  Before each write(2), the 
file offset is positioned at the end of the file, as if with lseek(2).  
The modification of the file offset and the write operation are 
performed as a single atomic step.
>> +	if (!fail)
>> +		tst_res(TPASS, "O_APPEND test passed.");
>> +	SAFE_CLOSE(fd);
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	write_buf[0] = SAFE_MALLOC(K2);
>> +	memset(write_buf[0], 0, K2);
>> +	write_buf[1] = SAFE_MALLOC(K1);
>> +	memset(write_buf[0], 1, K1);
>                           ^
> 			 1
>
> Also can you please switch these two buffers to a guarded buffers?
>
> See:
>
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#131-guarded-buffers
OK. I will switch to guarded buffers.
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	free(write_buf[0]);
>> +	free(write_buf[1]);
>> +
>> +	if (fd > -1)
> Probably fd != -1 would be a bit clearer.
Got it.
>> +		SAFE_CLOSE(fd);
>> +
>> +	SAFE_UNLINK(DATA_FILE);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.needs_tmpdir = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = verify_write,
>> +};
>> -- 
>> 1.8.3.1
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp