[LTP] [PATCH] open14: Rewrite to new API
Martin Doucha
mdoucha@suse.cz
Mon Jul 21 17:46:15 CEST 2025
On 21. 07. 25 15:56, Andrea Cervesato wrote:
> Hi!
>
> On 7/18/25 6:29 PM, Martin Doucha wrote:
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>> testcases/kernel/syscalls/open/open14.c | 276 +++++++++++-------------
>> 1 file changed, 131 insertions(+), 145 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/open/open14.c b/testcases/
>> kernel/syscalls/open/open14.c
>> index 3ecb7e4fb..41e274e69 100644
>> --- a/testcases/kernel/syscalls/open/open14.c
>> +++ b/testcases/kernel/syscalls/open/open14.c
>> @@ -1,63 +1,45 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> * Copyright (c) 2015-2016 Oracle and/or its affiliates. All Rights
>> Reserved.
>> - *
>> - * This program is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU General Public License as
>> - * published by the Free Software Foundation; either version 2 of
>> - * the License, or (at your option) any later version.
>> - *
>> - * 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. See the
>> - * GNU General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> - *
>> * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
>> - *
>> + * Copyright (c) 2025 SUSE LLC <mdoucha@suse.cz>
>> */
>> -#define _GNU_SOURCE
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <unistd.h>
>> -#include <errno.h>
>> +/*\
>> + * Check the functionality of O_TMPFILE flag for open() syscall.
> It would be nice to add the 3 test cases in the description.
OK, I'll send v2 soon.
>> + */
>> -#include "test.h"
>> -#include "safe_macros.h"
>> +#include "tst_test.h"
>> +#include "tst_safe_macros.h"
>> #include "lapi/fcntl.h"
>> -char *TCID = "open14";
>> -int TST_TOTAL = 3;
>> -static ssize_t size;
>> +#define FILE_COUNT 100
>> +#define MNTPOINT "mntpoint"
>> +
>> static char buf[1024];
>> +static int fds[FILE_COUNT];
> Since we are not using variants, we can initialize all items ad
> declaration without doing it inside the setup:
>
> static char buf[1024] ={1};
> static int fds[FILE_COUNT] = {-1};
>
> But probably buf can be initialized using tst_rand_data.
This would only initialize the first element and fill the rest of the
array with zeroes. Initializing the arrays in setup() is the correct
approach.
>> +static const ssize_t size = sizeof(buf);
> Why not defining a macro with the size of the buf and use that one
> inside the code?
>
> #define BUFSIZE 1024
There's no practical difference so I prefer to leave the original code
as is.
>> static const ssize_t blocks_num = 4;
>> -static struct stat st;
>> -
>> -static void cleanup(void)
>> -{
>> - tst_rmdir();
>> -}
>> static void setup(void)
>> {
>> - tst_tmpdir();
>> + int i;
> In c99 we can define int inside the for loop.
We can, but I chose not to.
>> - size = sizeof(buf);
>> + for (i = 0; i < FILE_COUNT; i++)
>> + fds[i] = -1;
>> memset(buf, 1, size);
>> + SAFE_CHDIR(MNTPOINT);
>> + fds[0] = open(".", O_TMPFILE | O_RDWR, 0600);
>> - int fd = open(".", O_TMPFILE | O_RDWR, 0600);
>> -
>> - if (fd == -1) {
>> + if (fds[0] == -1) {
>> if (errno == EISDIR || errno == ENOTSUP)
>> - tst_brkm(TCONF, cleanup, "O_TMPFILE not supported");
>> + tst_brk(TCONF, "O_TMPFILE not supported");
>> - tst_brkm(TBROK | TERRNO, cleanup, "open() failed");
>> + tst_brk(TBROK | TERRNO, "open() failed");
>> }
>> - SAFE_CLOSE(cleanup, fd);
>> + SAFE_CLOSE(fds[0]);
>> }
>> static void write_file(int fd)
>> @@ -65,183 +47,187 @@ static void write_file(int fd)
>> int i;
>> for (i = 0; i < blocks_num; ++i)
>> - SAFE_WRITE(cleanup, SAFE_WRITE_ALL, fd, buf, size);
>> + SAFE_WRITE(1, fd, buf, size);
>> }
>> void test01(void)
>> {
>> - int fd;
>> - char path[PATH_MAX], tmp[PATH_MAX];
>> -
>> - tst_resm(TINFO, "creating a file with O_TMPFILE flag");
>> - fd = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
>> -
>> - tst_resm(TINFO, "writing data to the file");
>> - write_file(fd);
>> + struct stat st;
>> + char path[PATH_MAX];
>> - SAFE_FSTAT(cleanup, fd, &st);
>> - tst_resm(TINFO, "file size is '%li'", (long)st.st_size);
>> + tst_res(TINFO, "Testing creation and linking of single temp file");
>> + fds[0] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
>> + write_file(fds[0]);
>> + SAFE_FSTAT(fds[0], &st);
>> if (st.st_size != blocks_num * size) {
>> - tst_resm(TFAIL, "not expected size: '%li' != '%zu'",
>> + tst_res(TFAIL, "Unexpected test file size: %li != %zu",
>> (long)st.st_size, blocks_num * size);
>> - SAFE_CLOSE(cleanup, fd);
>> - return;
>> + } else {
>> + tst_res(TPASS, "Test file size is %li", (long)st.st_size);
>> }
>> - tst_resm(TINFO, "looking for the file in '.'");
>> - if (!tst_dir_is_empty(cleanup, ".", 1))
>> - tst_brkm(TFAIL, cleanup, "found a file, this is not expected");
>> - tst_resm(TINFO, "file not found, OK");
>> + if (!tst_dir_is_empty(".", 1))
>> + tst_res(TFAIL, "Test directory is not empty");
>> + else
>> + tst_res(TPASS, "Test directory is empty");
>> - snprintf(path, PATH_MAX, "/proc/self/fd/%d", fd);
>> - SAFE_READLINK(cleanup, path, tmp, PATH_MAX);
>> + snprintf(path, PATH_MAX, "/proc/self/fd/%d", fds[0]);
>> + tst_res(TINFO, "Linking unnamed test file to 'tmpfile'");
>> + SAFE_LINKAT(AT_FDCWD, path, AT_FDCWD, "tmpfile", AT_SYMLINK_FOLLOW);
>> - tst_resm(TINFO, "renaming '%s' -> 'tmpfile'", tmp);
>> - SAFE_LINKAT(cleanup, AT_FDCWD, path, AT_FDCWD, "tmpfile",
>> - AT_SYMLINK_FOLLOW);
>> -
>> - if (tst_dir_is_empty(cleanup, ".", 1))
>> - tst_brkm(TFAIL, cleanup, "file not found");
>> + if (tst_dir_is_empty(".", 1)) {
>> + tst_res(TFAIL, "Test directory is still empty");
>> + SAFE_CLOSE(fds[0]);
>> + return;
>> + }
>> - SAFE_UNLINK(cleanup, "tmpfile");
>> - SAFE_CLOSE(cleanup, fd);
>> + if (access("tmpfile", F_OK)) {
>> + tst_res(TFAIL | TERRNO, "Linked test file not found");
>> + SAFE_CLOSE(fds[0]);
>> + return;
>> + }
>> - tst_resm(TPASS, "single file tests passed");
>> + tst_res(TPASS, "Test file was linked correctly");
>> + SAFE_UNLINK("tmpfile");
>> + SAFE_CLOSE(fds[0]);
>> }
>> -static void read_file(int fd)
>> +static int read_file(int fd)
>> {
>> int i;
>> char tmp[size];
>> - SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
>> + SAFE_LSEEK(fd, 0, SEEK_SET);
>> for (i = 0; i < blocks_num; ++i) {
>> - SAFE_READ(cleanup, 0, fd, tmp, size);
>> - if (memcmp(buf, tmp, size))
>> - tst_brkm(TFAIL, cleanup, "got unexepected data");
>> + SAFE_READ(0, fd, tmp, size);
>> +
>> + if (memcmp(buf, tmp, size)) {
>> + tst_res(TFAIL, "got unexepected data");
>> + return 1;
>> + }
>> }
>> +
>> + return 0;
>> }
>> static void test02(void)
>> {
>> - const int files_num = 100;
>> - int i, fd[files_num];
>> + int i, fails = 0;
>> char path[PATH_MAX];
>> - tst_resm(TINFO, "create files in multiple directories");
>> - for (i = 0; i < files_num; ++i) {
>> + tst_res(TINFO, "Testing temp files in multiple directories");
>> + for (i = 0; i < FILE_COUNT; ++i) {
>> snprintf(path, PATH_MAX, "tst02_%d", i);
>> - SAFE_MKDIR(cleanup, path, 0700);
>> - SAFE_CHDIR(cleanup, path);
>> -
>> - fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, 0600);
>> + SAFE_MKDIR(path, 0700);
>> + SAFE_CHDIR(path);
>> + fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, 0600);
>> }
>> - tst_resm(TINFO, "removing test directories");
>> - for (i = files_num - 1; i >= 0; --i) {
>> - SAFE_CHDIR(cleanup, "..");
>> + tst_res(TINFO, "Removing test directories");
>> + for (i = FILE_COUNT - 1; i >= 0; --i) {
>> + SAFE_CHDIR("..");
>> snprintf(path, PATH_MAX, "tst02_%d", i);
>> - SAFE_RMDIR(cleanup, path);
>> + SAFE_RMDIR(path);
>> }
>> - tst_resm(TINFO, "writing/reading temporary files");
>> - for (i = 0; i < files_num; ++i) {
>> - write_file(fd[i]);
>> - read_file(fd[i]);
>> + tst_res(TINFO, "Writing and reading temporary files");
>> + for (i = 0; i < FILE_COUNT; ++i) {
>> + write_file(fds[i]);
>> + fails += read_file(fds[i]);
>> }
>> - tst_resm(TINFO, "closing temporary files");
>> - for (i = 0; i < files_num; ++i)
>> - SAFE_CLOSE(cleanup, fd[i]);
>> + tst_res(TINFO, "Closing temporary files");
>> + for (i = 0; i < FILE_COUNT; ++i)
>> + SAFE_CLOSE(fds[i]);
>> - tst_resm(TPASS, "multiple files tests passed");
>> + if (!fails)
>> + tst_res(TPASS, "Multiple files test passed");
>> }
>> static void link_tmp_file(int fd)
>> {
>> char path1[PATH_MAX], path2[PATH_MAX];
>> - snprintf(path1, PATH_MAX, "/proc/self/fd/%d", fd);
>> - snprintf(path2, PATH_MAX, "tmpfile_%d", fd);
>> -
>> - SAFE_LINKAT(cleanup, AT_FDCWD, path1, AT_FDCWD, path2,
>> - AT_SYMLINK_FOLLOW);
>> + snprintf(path1, PATH_MAX, "/proc/self/fd/%d", fd);
>> + snprintf(path2, PATH_MAX, "tmpfile_%d", fd);
>> + SAFE_LINKAT(AT_FDCWD, path1, AT_FDCWD, path2, AT_SYMLINK_FOLLOW);
>> }
>> static void test03(void)
>> {
>> - const int files_num = 100;
>> const mode_t test_perms[] = { 0, 07777, 001, 0755, 0644, 0440 };
>> - int i, fd[files_num];
>> + int i, fails = 0;
>> char path[PATH_MAX];
>> struct stat st;
>> mode_t mask = umask(0), perm;
>> umask(mask);
>> + tst_res(TINFO, "Testing linked temp files access mode");
>> - tst_resm(TINFO, "create multiple directories, link files into
>> them");
>> - tst_resm(TINFO, "and check file permissions");
>> - for (i = 0; i < files_num; ++i) {
>> -
>> + for (i = 0; i < FILE_COUNT; ++i) {
>> snprintf(path, PATH_MAX, "tst03_%d", i);
>> - SAFE_MKDIR(cleanup, path, 0700);
>> - SAFE_CHDIR(cleanup, path);
>> + SAFE_MKDIR(path, 0700);
>> + SAFE_CHDIR(path);
>> perm = test_perms[i % ARRAY_SIZE(test_perms)];
>> -
>> - fd[i] = SAFE_OPEN(cleanup, ".", O_TMPFILE | O_RDWR, perm);
>> -
>> - write_file(fd[i]);
>> - read_file(fd[i]);
>> -
>> - link_tmp_file(fd[i]);
>> -
>> - snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
>> -
>> - SAFE_LSTAT(cleanup, path, &st);
>> -
>> - mode_t exp_mode = perm & ~mask;
>> -
>> - if ((st.st_mode & ~S_IFMT) != exp_mode) {
>> - tst_brkm(TFAIL, cleanup,
>> - "file mode read %o, but expected %o",
>> - st.st_mode & ~S_IFMT, exp_mode);
>> + fds[i] = SAFE_OPEN(".", O_TMPFILE | O_RDWR, perm);
>> + write_file(fds[i]);
>> + read_file(fds[i]);
>> + link_tmp_file(fds[i]);
>> +
>> + snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
>> + SAFE_LSTAT(path, &st);
>> + perm &= ~mask;
>> +
>> + if ((st.st_mode & ~S_IFMT) != perm) {
>> + tst_res(TFAIL, "Unexpected access mode: %04o != %04o",
>> + st.st_mode & ~S_IFMT, perm);
>> + fails++;
>> }
>> }
>> - tst_resm(TINFO, "remove files, directories");
>> - for (i = files_num - 1; i >= 0; --i) {
>> - snprintf(path, PATH_MAX, "tmpfile_%d", fd[i]);
>> - SAFE_UNLINK(cleanup, path);
>> - SAFE_CLOSE(cleanup, fd[i]);
>> + if (!fails)
>> + tst_res(TPASS, "File access modes are correct");
>> +
>> + tst_res(TINFO, "Removing files and directories");
>> - SAFE_CHDIR(cleanup, "..");
>> + for (i = FILE_COUNT - 1; i >= 0; --i) {
>> + snprintf(path, PATH_MAX, "tmpfile_%d", fds[i]);
>> + SAFE_UNLINK(path);
>> + SAFE_CLOSE(fds[i]);
>> + SAFE_CHDIR("..");
>> snprintf(path, PATH_MAX, "tst03_%d", i);
>> - SAFE_RMDIR(cleanup, path);
>> + SAFE_RMDIR(path);
>> }
>> -
>> - tst_resm(TPASS, "file permission tests passed");
>> }
>> -int main(int ac, char *av[])
>> +static void run(void)
>> {
>> - int lc;
>> -
>> - tst_parse_opts(ac, av, NULL, NULL);
>> + test01();
>> + test02();
>> + test03();
>> +}
>> - setup();
>> +static void cleanup(void)
>> +{
>> + int i;
>> - for (lc = 0; TEST_LOOPING(lc); ++lc) {
>> - tst_count = 0;
>> - test01();
>> - test02();
>> - test03();
>> + for (i = 0; i < FILE_COUNT; i++) {
>> + if (fds[i] >= 0)
>> + SAFE_CLOSE(fds[i]);
>> }
>> - cleanup();
>> - tst_exit();
>> + SAFE_CHDIR("..");
>> }
>> +
>> +static struct tst_test test = {
>> + .setup = setup,
>> + .test_all = run,
>> + .cleanup = cleanup,
>> + .mntpoint = MNTPOINT,
>> + .all_filesystems = 1
> We need .needs_root if we acquire a device.
Good catch, I'll fix it in v2.
>> +};
>
> The rest looks ok, but I'm wondering if we really need to call
> SAFE_CHDIR() so many times before SAFE_OPEN(".", ...) instead of passing
> the full path. We always obtain a file descriptor and in general we
> would like to reduce calls which are not 100% needed by the tests.
Feel free to submit a follow-up patch. But the API rewrite patch itself
should not change the original test scenario too much.
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
More information about the ltp
mailing list