<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Christian,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 25, 2019 at 9:20 PM Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">@ Li Wang<br>
<br>
Thanks for the review. I removed the testing of the basic<br>
functionalities in copy_file_range02 entirely because this is already<br>
done in great detail in the first test file. What's left is the error<br>
handling.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Nice work! V3 looks good except some extra suggests in below.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I also did not stumble upon the failure you found, but I think i have<br>
found the problem and corrected it:<br>
<br>
If the syscall is presented with a directory it returns EISDIR, if it is<br>
supposed to write in a read-only file it returns EBADF. I accidentally<br>
opened the directory as read-only and so both errors would by valid.<br>
<br>
In my case I always got EISDIR but that cannot be guaranteed to be the<br>
case on every system.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Your test case is good.</div><br></div><div><div class="gmail_default" style="font-size:small">I have confirmed that it's because my kernel doesn't has this commit:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default">commit 11cbfb10775aa2a01cee966d118049ede9d0bdf2</div><div class="gmail_default">Author: Amir Goldstein <<a href="mailto:amir73il@gmail.com" target="_blank">amir73il@gmail.com</a>></div><div class="gmail_default">Date: Tue Jan 31 10:34:56 2017 +0200</div><div class="gmail_default"><br></div><div class="gmail_default"> vfs: deny copy_file_range() for non regular files</div><div class="gmail_default"> </div><div class="gmail_default"> There is no in-tree file system that implements copy_file_range()</div><div class="gmail_default"> for non regular files.</div><div class="gmail_default"> </div><div class="gmail_default"> Deny an attempt to copy_file_range() a directory with EISDIR</div><div class="gmail_default"> and any other non regualr file with EINVAL to conform with</div><div class="gmail_default"> behavior of vfs_{clone,dedup}_file_range().</div><div class="gmail_default"> </div><div class="gmail_default"> This change is needed prior to converting sb_start_write()</div><div class="gmail_default"> to file_start_write() in the vfs helper.</div><div class="gmail_default" style="font-size:small"><br></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
The read-only flag is now removed.<br>
<br>
<br>
Kind regards,<br>
<br>
Christian<br>
<br>
<br>
On 25/04/2019 15:12, Christian Amann wrote:<br>
> The following tests are run for the syscall itself,<br>
> as well as the glibc implementation.<br>
><br>
> copy_file_range01:<br>
> restructured testcase, removed unnecessary code,<br>
> improved readability and shortened output (only<br>
> failures get printed now). This tests the basic<br>
> functionality of the syscall.<br>
><br>
> copy_file_range02:<br>
> add testcases which test basic error handling<br>
> of the syscall.<br>
><br>
> copy_file_range03:<br>
> add testcase to check if this operation updates<br>
> timestamps accordingly.<br>
><br>
> Signed-off-by: Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>><br></blockquote><div> </div><div><span class="gmail_default" style="font-size:small">Reviewed-by: Li Wang <<a href="mailto:liwang@redhat.com">liwang@redhat.com</a>></span></div><div><span class="gmail_default" style="font-size:small"></span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ---<br>
> <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> | 1 +<br>
> m4/ltp-copy_file_range.m4 | 7 +<br>
> runtest/syscalls | 2 +<br>
> .../kernel/syscalls/copy_file_range/.gitignore | 2 +<br>
> .../syscalls/copy_file_range/copy_file_range.h | 57 +++++<br>
> .../syscalls/copy_file_range/copy_file_range01.c | 239 +++++++++++----------<br>
> .../syscalls/copy_file_range/copy_file_range02.c | 105 +++++++++<br>
> .../syscalls/copy_file_range/copy_file_range03.c | 80 +++++++<br>
> 8 files changed, 377 insertions(+), 116 deletions(-)<br>
> create mode 100644 m4/ltp-copy_file_range.m4<br>
> create mode 100644 testcases/kernel/syscalls/copy_file_range/copy_file_range.h<br>
> create mode 100644 testcases/kernel/syscalls/copy_file_range/copy_file_range02.c<br>
> create mode 100644 testcases/kernel/syscalls/copy_file_range/copy_file_range03.c<br>
><br>
> diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> index fad8f8396..3fec68ede 100644<br>
> --- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> +++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
> @@ -197,6 +197,7 @@ LTP_CHECK_BUILTIN_CLEAR_CACHE<br>
> LTP_CHECK_CAPABILITY_SUPPORT<br>
> LTP_CHECK_CC_WARN_OLDSTYLE<br>
> LTP_CHECK_CLONE_SUPPORTS_7_ARGS<br>
> +LTP_CHECK_COPY_FILE_RANGE<br>
> LTP_CHECK_CRYPTO<br>
> LTP_CHECK_FIDEDUPE<br>
> LTP_CHECK_FORTIFY_SOURCE<br>
> diff --git a/m4/ltp-copy_file_range.m4 b/m4/ltp-copy_file_range.m4<br>
> new file mode 100644<br>
> index 000000000..2d87e8900<br>
> --- /dev/null<br>
> +++ b/m4/ltp-copy_file_range.m4<br>
> @@ -0,0 +1,7 @@<br>
> +dnl SPDX-License-Identifier: GPL-2.0-or-later<br>
> +dnl Copyright (c) 2019 SUSE LLC<br>
> +dnl Author: Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>><br>
> +<br>
> +AC_DEFUN([LTP_CHECK_COPY_FILE_RANGE],[<br>
> +AC_CHECK_FUNCS(copy_file_range,,)<br>
> +])<br>
> diff --git a/runtest/syscalls b/runtest/syscalls<br>
> index 2b8ca719b..33a70ee17 100644<br>
> --- a/runtest/syscalls<br>
> +++ b/runtest/syscalls<br>
> @@ -1561,6 +1561,8 @@ memfd_create03 memfd_create03<br>
> memfd_create04 memfd_create04<br>
> <br>
> copy_file_range01 copy_file_range01<br>
> +copy_file_range02 copy_file_range02<br>
> +copy_file_range03 copy_file_range03<br>
> <br>
> statx01 statx01<br>
> statx02 statx02<br>
> diff --git a/testcases/kernel/syscalls/copy_file_range/.gitignore b/testcases/kernel/syscalls/copy_file_range/.gitignore<br>
> index 6807420ef..e9e35f60f 100644<br>
> --- a/testcases/kernel/syscalls/copy_file_range/.gitignore<br>
> +++ b/testcases/kernel/syscalls/copy_file_range/.gitignore<br>
> @@ -1 +1,3 @@<br>
> /copy_file_range01<br>
> +/copy_file_range02<br>
> +/copy_file_range03<br>
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range.h b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h<br>
> new file mode 100644<br>
> index 000000000..d2ef1348b<br>
> --- /dev/null<br>
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h<br>
> @@ -0,0 +1,57 @@<br>
> +// SPDX-License-Identifier: GPL-2.0-or-later<br>
> +/*<br>
> + * Copyright (c) 2019 SUSE LLC<br>
> + * Author: Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>><br>
> + */<br>
> +<br>
> +#ifndef __COPY_FILE_RANGE_H__<br>
> +#define __COPY_FILE_RANGE_H__<br>
> +<br>
> +#include <stdbool.h><br>
> +#include <unistd.h><br>
> +#include "lapi/syscalls.h"<br>
> +<br>
> +#define TEST_VARIANTS 2<br>
> +<br>
> +#define MNTPOINT "mnt_point"<br>
> +#define FILE_SRC_PATH "file_src"<br>
> +#define FILE_DEST_PATH "file_dest"<br>
> +#define FILE_RDONL_PATH "file_rdonl"<br>
> +#define FILE_DIR_PATH "file_dir"<br>
> +#define FILE_MNTED_PATH MNTPOINT"/file_mnted"<br>
> +<br>
> +#define CONTENT "ABCDEFGHIJKLMNOPQRSTUVWXYZ12345\n"<br>
> +#define CONTSIZE (sizeof(CONTENT) - 1)<br>
> +<br>
> +static void syscall_info(void)<br>
> +{<br>
> + switch (tst_variant) {<br>
> + case 0:<br>
> + tst_res(TINFO, "Testing libc copy_file_range()");<br>
> + break;<br>
> + case 1:<br>
> + tst_res(TINFO, "Testing tst copy_file_range()");<br>
> + }<br>
> +}<br>
> +<br>
> +static int sys_copy_file_range(int fd_in, loff_t *off_in,<br>
> + int fd_out, loff_t *off_out, size_t len, unsigned int flags)<br>
> +{<br>
> + switch (tst_variant) {<br>
> +<br>
> + case 0:<br>
> +#ifdef HAVE_COPY_FILE_RANGE<br>
> + return copy_file_range(fd_in, off_in,<br>
> + fd_out, off_out, len, flags);<br>
> +#else<br>
> + tst_brk(TCONF, "libc copy_file_range() not supported!");<br>
> +#endif<br>
> + break;<br>
> + case 1:<br>
> + return tst_syscall(__NR_copy_file_range, fd_in, off_in, fd_out,<br>
> + off_out, len, flags);<br>
> + }<br>
> + return -1;<br>
> +}<br>
> +<br>
> +#endif /* __COPY_FILE_RANGE_H__ */<br>
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c<br>
> index 61a6042d9..e2000cf5a 100644<br>
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c<br>
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c<br>
> @@ -1,66 +1,30 @@<br>
> +// SPDX-License-Identifier: GPL-2.0-or-later<br>
> /*<br>
> - * Copyright (c) Linux Test Project, 2017<br>
> - *<br>
> - * This program is free software; you can redistribute it and/or modify<br>
> - * it under the terms of the GNU General Public License as published by<br>
> - * the Free Software Foundation; either version 2 of the License, or<br>
> - * (at your option) any later version.<br>
> + * Copyright (c) Linux Test Project, 2019<br>
> + */<br>
> +<br>
> +/*<br>
> + * This tests the fundamental functionalities of the copy_file_range<br>
> + * syscall. It does so by copying the contents of one file into<br>
> + * another using various different combinations for length and<br>
> + * input/output offsets.<br>
> *<br>
> - * This program is distributed in the hope that it will be useful,<br>
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See<br>
> - * the GNU General Public License for more details.<br>
> + * After a copy is done this test checks if the contents of both files<br>
> + * are equal at the given offsets. It is also inspected if the offsets<br>
> + * of the file descriptors are advanced correctly.<br>
> */<br>
> <br>
> #define _GNU_SOURCE<br>
> +<br>
> #include <stdio.h><br>
> -#include <errno.h><br>
> #include <stdlib.h><br>
> #include "tst_test.h"<br>
> #include "tst_safe_stdio.h"<br>
> -#include "lapi/syscalls.h"<br>
> -<br>
> -#define TEST_FILE_1 "copy_file_range_ltp01.txt"<br>
> -#define TEST_FILE_2 "copy_file_range_ltp02.txt"<br>
> -#define STR "abcdefghijklmnopqrstuvwxyz12345\n"<br>
> -<br>
> -#define verbose 0<br>
> -<br>
> -static size_t *len_arr;<br>
> -static loff_t **off_arr;<br>
> -static int len_sz, off_sz;<br>
> +#include "copy_file_range.h"<br>
> <br>
> -static void setup(void)<br>
> -{<br>
> - int i, fd, page_size;<br>
> -<br>
> - page_size = getpagesize();<br>
> -<br>
> - fd = SAFE_OPEN(TEST_FILE_1, O_RDWR | O_CREAT, 0664);<br>
> - /* Writing page_size * 4 of data into test file */<br>
> - for (i = 0; i < (int)(page_size * 4); i++)<br>
> - SAFE_WRITE(1, fd, STR, strlen(STR));<br>
> - SAFE_CLOSE(fd);<br>
> -<br>
> - len_sz = 4;<br>
> - len_arr = malloc(sizeof(size_t) * len_sz);<br>
> - len_arr[0] = 11;<br>
> - len_arr[1] = page_size - 1;<br>
> - len_arr[2] = page_size;<br>
> - len_arr[3] = page_size + 1;<br>
> -<br>
> - off_sz = 6;<br>
> - off_arr = malloc(sizeof(loff_t *) * off_sz);<br>
> - for (i = 1; i < off_sz; i++)<br>
> - off_arr[i] = malloc(sizeof(loff_t));<br>
> -<br>
> - off_arr[0] = NULL;<br>
> - *off_arr[1] = 0;<br>
> - *off_arr[2] = 17;<br>
> - *off_arr[3] = page_size - 1;<br>
> - *off_arr[4] = page_size;<br>
> - *off_arr[5] = page_size + 1;<br>
> -}<br>
> +static int page_size;<br>
> +static int errcount, numcopies;<br>
> +static int fd_in, fd_out;<br>
> <br>
> static int check_file_content(const char *fname1, const char *fname2,<br>
> loff_t *off1, loff_t *off2, size_t len)<br>
> @@ -90,52 +54,37 @@ static int check_file_content(const char *fname1, const char *fname2,<br>
> }<br>
> <br>
> static int check_file_offset(const char *m, int fd, loff_t len,<br>
> - loff_t *off_ori, loff_t *off_after)<br>
> + loff_t *off_before, loff_t *off_after)<br>
> {<br>
> + loff_t fd_off = SAFE_LSEEK(fd, 0, SEEK_CUR);<br>
> int ret = 0;<br>
> <br>
> - if (off_ori) {<br>
> - /* FD should stay untouched, and off_in/out is updated */<br>
> - loff_t fd_off = SAFE_LSEEK(fd, 0, SEEK_CUR);<br>
> -<br>
> - if (fd_off == 0) {<br>
> - if (verbose)<br>
> - tst_res(TPASS, "%s FD offset unchanged", m);<br>
> - } else {<br>
> - tst_res(TFAIL, "%s FD offset changed: %ld",<br>
> + if (off_before) {<br>
> + /*<br>
> + * copy_file_range offset is given:<br>
> + * - fd offset should stay 0,<br>
> + * - copy_file_range offset is updated<br>
> + */<br>
> + if (fd_off != 0) {<br>
> + tst_res(TFAIL,<br>
> + "%s fd offset unexpectedly changed: %ld",<br>
> m, (long)fd_off);<br>
> ret = 1;<br>
> - }<br>
> <br>
> - if (!off_after) {<br>
> - tst_res(TFAIL, "%s offset is NULL", m);<br>
> - ret = 1;<br>
> - }<br>
> -<br>
> - if ((off_after) && (*off_ori + len == *off_after)) {<br>
> - if (verbose) {<br>
> - tst_res(TPASS, "%s offset advanced as"<br>
> - " expected: %ld", m, (long)*off_after);<br>
> - }<br>
> - } else {<br>
> + } else if (*off_before + len != *off_after) {<br>
> tst_res(TFAIL, "%s offset unexpected value: %ld",<br>
> m, (long)*off_after);<br>
> ret = 1;<br>
> }<br>
> - } else {<br>
> - /* FD offset is advanced by len */<br>
> - loff_t fd_off = SAFE_LSEEK(fd, 0, SEEK_CUR);<br>
> -<br>
> - if (fd_off == len) {<br>
> - if (verbose) {<br>
> - tst_res(TPASS, "%s FD offset changed as"<br>
> - " expected: %ld", m, (long)fd_off);<br>
> - }<br>
> - } else {<br>
> - tst_res(TFAIL, "%s FD offset unexpected value: %ld",<br>
> + }<br>
> + /*<br>
> + * no copy_file_range offset given:<br>
> + * - fd offset advanced by length<br>
> + */<br>
> + else if (fd_off != len) {<br>
> + tst_res(TFAIL, "%s fd offset unexpected value: %ld",<br>
> m, (long)fd_off);<br>
> - ret = 1;<br>
> - }<br>
> + ret = 1;<br>
> }<br>
> <br>
> return ret;<br>
> @@ -143,77 +92,135 @@ static int check_file_offset(const char *m, int fd, loff_t len,<br>
> <br>
> static void test_one(size_t len, loff_t *off_in, loff_t *off_out)<br>
> {<br>
> + int ret;<br>
> size_t to_copy = len;<br>
> - int fd_in, fd_out, ret;<br>
> - loff_t *off_in_ori = off_in;<br>
> - loff_t *off_out_ori = off_out;<br>
> - loff_t off_in_copy;<br>
> - loff_t off_out_copy;<br>
> + loff_t off_in_value_copy, off_out_value_copy;<br>
> + loff_t *off_new_in = &off_in_value_copy;<br>
> + loff_t *off_new_out = &off_out_value_copy;<br>
> char str_off_in[32], str_off_out[32];<br>
> <br>
> if (off_in) {<br>
> - off_in_copy = *off_in;<br>
> - off_in = &off_in_copy;<br>
> + off_in_value_copy = *off_in;<br>
> sprintf(str_off_in, "%ld", (long)*off_in);<br>
> } else {<br>
> + off_new_in = NULL;<br>
> strcpy(str_off_in, "NULL");<br>
> }<br>
> <br>
> if (off_out) {<br>
> - off_out_copy = *off_out;<br>
> - off_out = &off_out_copy;<br>
> + off_out_value_copy = *off_out;<br>
> sprintf(str_off_out, "%ld", (long)*off_out);<br>
> } else {<br>
> + off_new_out = NULL;<br>
> strcpy(str_off_out, "NULL");<br>
> }<br>
> <br>
> - fd_in = SAFE_OPEN(TEST_FILE_1, O_RDONLY);<br>
> - fd_out = SAFE_OPEN(TEST_FILE_2, O_CREAT | O_WRONLY | O_TRUNC, 0644);<br>
> -<br>
> /*<br>
> * copy_file_range() will return the number of bytes copied between<br>
> * files. This could be less than the length originally requested.<br>
> */<br>
> do {<br>
> - TEST(tst_syscall(__NR_copy_file_range, fd_in, off_in, fd_out,<br>
> - off_out, to_copy, 0));<br>
> + TEST(sys_copy_file_range(fd_in, off_new_in, fd_out,<br>
> + off_new_out, to_copy, 0));<br>
> if (TST_RET == -1) {<br>
> tst_res(TFAIL | TTERRNO, "copy_file_range() failed");<br>
> - SAFE_CLOSE(fd_in);<br>
> - SAFE_CLOSE(fd_out);<br>
> + errcount++;<br>
> return;<br>
> }<br>
> <br>
> to_copy -= TST_RET;<br>
> } while (to_copy > 0);<br>
> <br>
> - ret = check_file_content(TEST_FILE_1, TEST_FILE_2,<br>
> - off_in_ori, off_out_ori, len);<br>
> - if (ret)<br>
> + ret = check_file_content(FILE_SRC_PATH, FILE_DEST_PATH,<br>
> + off_in, off_out, len);<br>
> + if (ret) {<br>
> tst_res(TFAIL, "file contents do not match");<br>
> + errcount++;<br>
> + return;<br>
> + }<br>
> <br>
> - ret |= check_file_offset("(in)", fd_in, len, off_in_ori, off_in);<br>
> - ret |= check_file_offset("(out)", fd_out, len, off_out_ori, off_out);<br>
> + ret |= check_file_offset("(in)", fd_in, len, off_in, off_new_in);<br>
> + ret |= check_file_offset("(out)", fd_out, len, off_out, off_new_out);<br>
> +<br>
> + if (ret != 0) {<br>
> + tst_res(TFAIL, "off_in: %s, off_out: %s, len: %ld",<br>
> + str_off_in, str_off_out, (long)len);<br>
> + errcount++;<br>
> + }<br>
> +}<br>
> <br>
> - tst_res(ret == 0 ? TPASS : TFAIL, "off_in: %s, off_out: %s, len: %ld",<br>
> - str_off_in, str_off_out, (long)len);<br>
> +static void open_files(void)<br>
> +{<br>
> + fd_in = SAFE_OPEN(FILE_SRC_PATH, O_RDONLY);<br>
> + fd_out = SAFE_OPEN(FILE_DEST_PATH, O_CREAT | O_WRONLY | O_TRUNC, 0644);<br>
> +}<br>
> <br>
> - SAFE_CLOSE(fd_in);<br>
> - SAFE_CLOSE(fd_out);<br>
> +static void close_files(void)<br>
> +{<br>
> + if (fd_out > 0)<br>
> + SAFE_CLOSE(fd_out);<br>
> + if (fd_in > 0)<br>
> + SAFE_CLOSE(fd_in);<br>
> }<br>
> <br>
> static void copy_file_range_verify(void)<br>
> {<br>
> int i, j, k;<br>
> <br>
> - for (i = 0; i < len_sz; i++)<br>
> - for (j = 0; j < off_sz; j++)<br>
> - for (k = 0; k < off_sz; k++)<br>
> + errcount = numcopies = 0;<br>
> + size_t len_arr[] = {11, page_size-1, page_size, page_size+1};<br>
> + loff_t off_arr_values[] = {0, 17, page_size-1, page_size, page_size+1};<br>
> +<br>
> + int num_offsets = ARRAY_SIZE(off_arr_values) + 1;<br>
> + loff_t *off_arr[num_offsets];<br>
> +<br>
> + off_arr[0] = NULL;<br>
> + for (i = 1; i < num_offsets; i++)<br>
> + off_arr[i] = &off_arr_values[i-1];<br>
> +<br>
> + /* Test all possible cobinations of given lengths and offsets */<br>
> + for (i = 0; i < (int)ARRAY_SIZE(len_arr); i++)<br>
> + for (j = 0; j < num_offsets; j++)<br>
> + for (k = 0; k < num_offsets; k++) {<br>
> + open_files();<br>
> test_one(len_arr[i], off_arr[j], off_arr[k]);<br>
> + close_files();<br>
> + numcopies++;<br>
> + }<br>
> +<br>
> + if (errcount == 0)<br>
> + tst_res(TPASS,<br>
> + "copy_file_range completed all %d copy jobs successfully!",<br>
> + numcopies);<br>
> + else<br>
> + tst_res(TINFO, "copy_file_range failed %d of %d copy jobs.",<br>
> + errcount, numcopies);<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">we should use TFAIL but not TINFO for the failure report.</div><div class="gmail_default"><a href="https://github.com/linux-test-project/ltp/blob/master/doc/test-writing-guidelines.txt#L414">https://github.com/linux-test-project/ltp/blob/master/doc/test-writing-guidelines.txt#L414</a><br></div><div class="gmail_default"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +}<br>
> +<br>
> +static void setup(void)<br>
> +{<br>
> + int i, fd;<br>
> +<br>
> + syscall_info();<br>
> +<br>
> + page_size = getpagesize();<br>
> +<br>
> + fd = SAFE_OPEN(FILE_SRC_PATH, O_RDWR | O_CREAT, 0664);<br>
> + /* Writing page_size * 4 of data into test file */<br>
> + for (i = 0; i < (int)(page_size * 4); i++)<br>
> + SAFE_WRITE(1, fd, CONTENT, CONTSIZE);<br>
> + SAFE_CLOSE(fd);<br>
> +}<br>
> +<br>
> +static void cleanup(void)<br>
> +{<br>
> + close_files();<br>
> }<br>
> <br>
> static struct tst_test test = {<br>
> .setup = setup,<br>
> + .cleanup = cleanup,<br>
> .needs_tmpdir = 1,<br>
> .test_all = copy_file_range_verify,<br>
> + .test_variants = TEST_VARIANTS,<br>
> };<br>
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c<br>
> new file mode 100644<br>
> index 000000000..a9fd713aa<br>
> --- /dev/null<br>
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c<br>
> @@ -0,0 +1,105 @@<br>
> +// SPDX-License-Identifier: GPL-2.0-or-later<br>
> +/*<br>
> + * Copyright (c) 2019 SUSE LLC<br>
> + * Author: Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>><br>
> + */<br>
> +<br>
> +/*<br>
> + * Tests basic error handling of the<br>
> + * copy_file_range syscall<br>
> + *<br>
> + * 1) Try to copy contents to file open as readonly<br>
> + * -> EBADF<br>
> + * 2) Try to copy contents to file on different mounted<br>
> + * filesystem -> EXDEV<br>
> + * 3) Try to copy contents to directory -> EISDIR<br>
> + * 4) Try to copy contents to a file opened with the<br>
> + * O_APPEND flag -> EBADF<br>
> + */<br>
> +<br>
> +#define _GNU_SOURCE<br>
> +<br>
> +#include "tst_test.h"<br>
> +#include "copy_file_range.h"<br>
> +<br>
> +static int fd_src;<br>
> +static int fd_rdonly;<br>
> +static int fd_mnted;<br>
> +static int fd_dir;<br>
> +static int fd_append;<br>
> +<br>
> +static struct tcase {<br>
> + int *copy_to_fd;<br>
> + int exp_err;<br>
> +} tcases[] = {<br>
> + {&fd_rdonly, EBADF},<br>
> + {&fd_mnted, EXDEV},<br>
> + {&fd_dir, EISDIR},<br>
> + {&fd_append, EBADF},<br>
> +};<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">I'm thinking maybe we can try test some more invalid argurments(e.g flags) here?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default">static struct tcase {</div><div class="gmail_default"><span style="white-space:pre"> </span>int<span style="white-space:pre"> </span>*copy_to_fd;</div><div class="gmail_default"><span style="white-space:pre"> </span>int<span style="white-space:pre"> </span>flags;</div><div class="gmail_default"><span style="white-space:pre"> </span>int<span style="white-space:pre"> </span>exp_err;</div><div class="gmail_default">} tcases[] = {</div><div class="gmail_default"><span style="white-space:pre"> </span>{&fd_rdonly, 0, EBADF},</div><div class="gmail_default"><span style="white-space:pre"> </span>{&fd_mnted, 0, EXDEV},</div><div class="gmail_default"><span style="white-space:pre"> </span>{&fd_dir, 0, EISDIR},</div><div class="gmail_default"><span style="white-space:pre"> </span>{&fd_append, 0, EBADF},</div><div class="gmail_default"><span style="white-space:pre"> </span>{&fd_append, -1, EINVAL},</div><div class="gmail_default">};</div><div class="gmail_default"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +static void verify_copy_file_range(unsigned int n)<br>
> +{<br>
> + struct tcase *tc = &tcases[n];<br>
> +<br>
> + TEST(sys_copy_file_range(fd_src, 0, *tc->copy_to_fd, 0, CONTSIZE, 0));<br>
> +<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">I would like change the error handler logic as below to make something easier to understand:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"><div class="gmail_default">if (TST_RET == -1) {</div><div class="gmail_default"> if (tc->exp_err == TST_ERR) {</div><div class="gmail_default"> tst_res(TPASS | TTERRNO, "copy_file_range failed as expected");</div><div class="gmail_default"> } else {</div><div class="gmail_default"> tst_res(TFAIL | TTERRNO,</div><div class="gmail_default"> "copy_file_range failed unexpectedly; expected %s, but got",</div><div class="gmail_default"> tst_strerrno(tc->exp_err));</div><div class="gmail_default"> }</div><div class="gmail_default">} else {</div><div class="gmail_default"> tst_res(TFAIL,</div><div class="gmail_default"> "copy_file_range returned wrong value: %ld", TST_RET);</div><div class="gmail_default">}</div></div><div class="gmail_default"><span style="white-space:pre-wrap"><br></span></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + if (tc->exp_err != TST_ERR) {<br>
> + tst_res(TFAIL,<br>
> + "copy_file_range failed with %s, expected %s",<br>
> + tst_strerrno(TST_ERR), tst_strerrno(tc->exp_err));<br>
> + return;<br>
> + }<br>
> +<br>
> + if (TST_RET != -1) {<br>
> + tst_res(TFAIL,<br>
> + "copy_file_range returned wrong value: %ld", TST_RET);<br>
> + return;<br>
> + }<br>
> +<br>
> + tst_res(TPASS, "copy_file_range ended with %s as expected",<br>
> + tst_strerrno(tc->exp_err));<br>
> +}<br>
> +<br>
> +static void cleanup(void)<br>
> +{<br>
> + if (fd_append > 0)<br>
> + SAFE_CLOSE(fd_append);<br>
> + if (fd_dir > 0)<br>
> + SAFE_CLOSE(fd_dir);<br>
> + if (fd_mnted > 0)<br>
> + SAFE_CLOSE(fd_mnted);<br>
> + if (fd_rdonly > 0)<br>
> + SAFE_CLOSE(fd_rdonly);<br>
> + if (fd_src > 0)<br>
> + SAFE_CLOSE(fd_src);<br>
> +}<br>
> +<br>
> +static void setup(void)<br>
> +{<br>
> + syscall_info();<br>
> +<br>
> + if (access(FILE_DIR_PATH, F_OK) == -1)<br>
> + SAFE_MKDIR(FILE_DIR_PATH, 0777);<br>
> +<br>
> + fd_src = SAFE_OPEN(FILE_SRC_PATH, O_RDWR | O_CREAT, 0664);<br>
> + fd_rdonly = SAFE_OPEN(FILE_RDONL_PATH, O_RDONLY | O_CREAT, 0664);<br>
> + fd_mnted = SAFE_OPEN(FILE_MNTED_PATH, O_RDWR | O_CREAT, 0664);<br>
> + fd_dir = SAFE_OPEN(FILE_DIR_PATH, O_DIRECTORY);<br>
> + fd_append = SAFE_OPEN(<br>
> + FILE_DEST_PATH, O_RDWR | O_CREAT | O_APPEND, 0664);<br>
> +<br>
> + SAFE_WRITE(1, fd_src, CONTENT, CONTSIZE);<br>
> +}<br>
> +<br>
> +static struct tst_test test = {<br>
> + .test = verify_copy_file_range,<br>
> + .tcnt = ARRAY_SIZE(tcases),<br>
> + .setup = setup,<br>
> + .cleanup = cleanup,<br>
> + .needs_root = 1,<br>
> + .mount_device = 1,<br>
> + .mntpoint = MNTPOINT,<br>
> + .dev_fs_type = "ext4",<br>
> + .test_variants = TEST_VARIANTS,<br>
> +};<br>
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c<br>
> new file mode 100644<br>
> index 000000000..d2c51632b<br>
> --- /dev/null<br>
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c<br>
> @@ -0,0 +1,80 @@<br>
> +// SPDX-License-Identifier: GPL-2.0-or-later<br>
> +/*<br>
> + * Copyright (c) 2019 SUSE LLC<br>
> + * Author: Christian Amann <<a href="mailto:camann@suse.com" target="_blank">camann@suse.com</a>><br>
> + */<br>
> +<br>
> +/*<br>
> + * Copies the contents of one file into another and<br>
> + * checks if the timestamp gets updated in the process.<br>
> + */<br>
> +<br>
> +#define _GNU_SOURCE<br>
> +<br>
> +#include "tst_test.h"<br>
> +#include "copy_file_range.h"<br>
> +<br>
> +static int fd_src;<br>
> +static int fd_dest;<br>
> +<br>
> +unsigned long getTimestamp(int fd)<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">To avoid camelcase naming in LTP, how about get_timestamp() ?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +{<br>
> + struct stat filestat;<br>
> +<br>
> + fstat(fd, &filestat);<br>
> + return filestat.st_mtime;<br>
> +}<br>
> +<br>
> +static void verify_copy_file_range_timestamp(void)<br>
> +{<br>
> + loff_t offset;<br>
> + unsigned long timestamp, updated_timestamp;<br>
> +<br>
> + timestamp = getTimestamp(fd_dest);<br>
> + usleep(1000000);<br>
> +<br>
> + offset = 0;<br>
> + TEST(sys_copy_file_range(fd_src, &offset,<br>
> + fd_dest, 0, CONTSIZE, 0));<br>
> + if (TST_RET == -1) {<br>
> + tst_res(TFAIL, "copy_file_range unexpectedly failed!");<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Seems tst_brk(TBROK | TTERRNO, ...) is better here, then we can remove the return sentence.</div><div class="gmail_default" style="font-size:small"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + return;<br>
> + }<br>
> +<br>
> + updated_timestamp = getTimestamp(fd_dest);<br>
> +<br>
> + if (timestamp == updated_timestamp) {<br>
> + tst_res(TFAIL, "copy_file_range did not update timestamp!");<br>
> + return;</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + }<br>
> +<br>
> + tst_res(TPASS, "copy_file_range sucessfully updated the timestamp.");<br>
> +}<br>
> +<br>
> +static void cleanup(void)<br>
> +{<br>
> + if (fd_dest > 0)<br>
> + SAFE_CLOSE(fd_dest);<br>
> + if (fd_src > 0)<br>
> + SAFE_CLOSE(fd_src);<br>
> +}<br>
> +<br>
> +static void setup(void)<br>
> +{<br>
> + syscall_info();<br>
> +<br>
> + fd_dest = SAFE_OPEN(FILE_DEST_PATH, O_RDWR | O_CREAT, 0664);<br>
> + fd_src = SAFE_OPEN(FILE_SRC_PATH, O_RDWR | O_CREAT, 0664);<br>
> + SAFE_WRITE(1, fd_src, CONTENT, CONTSIZE);<br>
> + SAFE_CLOSE(fd_src);<br>
> + fd_src = SAFE_OPEN(FILE_SRC_PATH, O_RDONLY);<br>
> +}<br>
> +<br>
> +<br>
> +static struct tst_test test = {<br>
> + .test_all = verify_copy_file_range_timestamp,<br>
> + .setup = setup,<br>
> + .cleanup = cleanup,<br>
> + .needs_tmpdir = 1,<br>
> + .test_variants = TEST_VARIANTS,<br>
> +};<br>
<br>
<br>
-- <br>
Mailing list info: <a href="https://lists.linux.it/listinfo/ltp" rel="noreferrer" target="_blank">https://lists.linux.it/listinfo/ltp</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail-m_-839912634004947555gmail-m_589771061953689425gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div></div></div></div></div></div></div></div></div></div></div>