[LTP] [PATCH v2 1/3] fchownat01: Convert to new API
Andrea Cervesato
andrea.cervesato@suse.com
Wed Jan 15 13:16:09 CET 2025
Hi!
On 1/15/25 10:12, Shiyang Ruan via ltp wrote:
> 1. convert to new API
> 2. move error tests to fchownat03
> 3. check if uid/gid is correctly set
>
> ---
> V1: https://lore.kernel.org/ltp/20240126062540.2596279-1-ruansy.fnst@fujitsu.com/T/#u
>
> Changes:
> * Move error tests from fchownat01 to fchownat03
> * fchownat01: Add check if file's ownership changed successfully
> * fchownat02: Use AT_FDCWD instead of dir_fd
>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> ---
> .../kernel/syscalls/fchownat/fchownat01.c | 160 ++++++------------
> 1 file changed, 50 insertions(+), 110 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fchownat/fchownat01.c b/testcases/kernel/syscalls/fchownat/fchownat01.c
> index 7771c111a..6768fdd77 100644
> --- a/testcases/kernel/syscalls/fchownat/fchownat01.c
> +++ b/testcases/kernel/syscalls/fchownat/fchownat01.c
> @@ -1,133 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - * Copyright (c) International Business Machines Corp., 2006
> - * AUTHOR: Yi Yang <yyangcdl@cn.ibm.com>
> - *
> - * 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 will 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, write to the Free Software Foundation,
> - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) International Business Machines Corp., 2006
> + * Copyright (c) Linux Test Project, 2007-2025
> */
> -/*
> - * DESCRIPTION
> - * This test case will verify basic function of fchownat
> - * added by kernel 2.6.16 or up.
> +
> +/*\
> + * [Description]
> + *
> + * Verify basic function of fchownat() added by kernel 2.6.16 or up.
We need a bit more information here. The description should tell at
first read what test is doing without reading the source code.
> */
>
> #define _GNU_SOURCE
> +#include "tst_test.h"
>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <stdlib.h>
> -#include <errno.h>
> -#include <string.h>
> -#include <signal.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> -#include "lapi/fcntl.h"
> -
> -#define TESTFILE "testfile"
> -
> -static void setup(void);
> -static void cleanup(void);
> +#define TESTFILE1 "testfile1"
> +#define TESTFILE2 "testfile2"
>
> +static uid_t set_uid = 1000;
> +static gid_t set_gid = 1000;
> static int dir_fd;
> -static int fd;
> -static int no_fd = -1;
> static int cu_fd = AT_FDCWD;
>
> -static struct test_case_t {
> - int exp_ret;
> - int exp_errno;
> - int flag;
> - int *fds;
> - char *filenames;
> -} test_cases[] = {
> - {0, 0, 0, &dir_fd, TESTFILE},
> - {-1, ENOTDIR, 0, &fd, TESTFILE},
> - {-1, EBADF, 0, &no_fd, TESTFILE},
> - {-1, EINVAL, 9999, &dir_fd, TESTFILE},
> - {0, 0, 0, &cu_fd, TESTFILE},
> +static struct tcase {
> + int *fd;
> + char *filename;
> +} tcases[] = {
> + { &dir_fd, TESTFILE1, },
> + { &cu_fd, TESTFILE2, },
> };
>
> -char *TCID = "fchownat01";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -static void fchownat_verify(const struct test_case_t *);
> -
> -int main(int ac, char **av)
> +static void fchownat_verify(unsigned int n)
> {
> - int lc;
> - int i;
> -
> - tst_parse_opts(ac, av, NULL, NULL);
> -
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> - tst_count = 0;
> - for (i = 0; i < TST_TOTAL; i++)
> - fchownat_verify(&test_cases[i]);
> - }
> -
> - cleanup();
> - tst_exit();
> + struct tcase *tc = &tcases[n];
> + struct stat stat_buf;
> + int fd = *tc->fd;
> + const char *filename = tc->filename;
> +
> + TST_EXP_PASS_SILENT(fchownat(fd, filename, set_uid, set_gid, 0));
We can use the regular TST_EXP_PASS() since we are testing fchownat()
syscall.
> +
> + /* check if the file's ownership changed successfully */
comment is not needed.
> + SAFE_STAT(filename, &stat_buf);
> + if (stat_buf.st_uid == set_uid && stat_buf.st_gid == set_gid)
> + tst_res(TPASS, "fchownat(%d, %s, %d, %d, 0)",
> + fd, filename, set_uid, set_gid);
> + else
> + tst_res(TFAIL, "fchownat() fails because uid(%d) != set(%d) || "
> + "gid(%d) != set(%d)",
> + stat_buf.st_uid, set_uid,
> + stat_buf.st_gid, set_gid);
We can simplify with:
TST_EXP_EQ_LI(stat_buf.st_uid, set_uid);
TST_EXP_EQ_LI(stat_buf.st_gid, set_gid);
> }
>
> static void setup(void)
> {
> - tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> - TEST_PAUSE;
> -
> - tst_tmpdir();
> -
> - dir_fd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
> -
> - SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
> -
> - fd = SAFE_OPEN(cleanup, "testfile2", O_CREAT | O_RDWR, 0600);
> -}
> -
> -static void fchownat_verify(const struct test_case_t *test)
> -{
> - TEST(fchownat(*(test->fds), test->filenames, geteuid(),
> - getegid(), test->flag));
> -
> - if (TEST_RETURN != test->exp_ret) {
> - tst_resm(TFAIL | TTERRNO,
> - "fchownat() returned %ld, expected %d, errno=%d",
> - TEST_RETURN, test->exp_ret, test->exp_errno);
> - return;
> - }
> -
> - if (TEST_ERRNO == test->exp_errno) {
> - tst_resm(TPASS | TTERRNO,
> - "fchownat() returned the expected errno %d: %s",
> - test->exp_ret, strerror(test->exp_errno));
> - } else {
> - tst_resm(TFAIL | TTERRNO,
> - "fchownat() failed unexpectedly; expected: %d - %s",
> - test->exp_errno, strerror(test->exp_errno));
> - }
> + dir_fd = SAFE_OPEN("./", O_DIRECTORY);
> + SAFE_TOUCH(TESTFILE1, 0600, NULL);
> + SAFE_TOUCH(TESTFILE2, 0600, NULL);
> }
>
> static void cleanup(void)
> {
> - if (fd > 0)
> - close(fd);
> -
> if (dir_fd > 0)
It's better to use dir_fd != -1 and initialize dir_fd to -1 at definition.
> - close(dir_fd);
> -
> - tst_rmdir();
> + SAFE_CLOSE(dir_fd);
> }
> +
> +static struct tst_test test = {
> + .needs_tmpdir = 1,
> + .needs_root = 1,
> + .test = fchownat_verify,
> + .tcnt = ARRAY_SIZE(tcases),
> + .setup = setup,
> + .cleanup = cleanup,
> +};
I have the impression we are over-complicating things using test case
struct here. We have only 2 use cases: one with directory descriptor and
one with AT_FDCWD.
We can just add both testcases inside the verify_fchownat() function and
initialize file descriptors inside setup(). In this way we reduce the
amount of code and we simplify the overall structure of the test.
Kind regards,
Andrea
More information about the ltp
mailing list