Menu
▾
▴
Re: [LTP] [PATCH v2 1/2] fchownat/fchownat01.c: cleanup
|
From: <ch...@su...> - 2014-02-25 15:11:39
|
Hi!
> #define _GNU_SOURCE
>
> @@ -53,38 +33,40 @@
> #include <errno.h>
> #include <string.h>
> #include <signal.h>
> +
> #include "test.h"
> #include "usctest.h"
> -#include "linux_syscall_numbers.h"
> -
> -#define TEST_CASES 6
> -#ifndef AT_FDCWD
> -#define AT_FDCWD -100
> -#endif
You should include the "lapi/fcntl.h" instead of the defines.
> -void setup();
> -void cleanup();
> -void setup_every_copy();
> +#include "safe_macros.h"
> +#include "fchownat.h"
> +
> +#define TESTFILE "testfile"
> +#define TESTFILE2 "testfile2"
> +
> +static void setup(void);
> +static void cleanup(void);
> +
> +static int dirfd;
> +static int fd;
> +static int no_fd = -1;
> +static int cu_fd = AT_FDCWD;
> +
> +static struct test_case_t {
> + int ret;
> + int flag;
> + int *fds;
> + char *filenames;
> +} test_cases[] = {
> + {0, 0, &dirfd, TESTFILE},
> + {0, 0, &dirfd, TESTFILE2},
These test are identical, aren't they?
> + {ENOTDIR, 0, &fd, TESTFILE},
> + {EBADF, 0, &no_fd, TESTFILE},
> + {EINVAL, 9999, &dirfd, TESTFILE},
> + {0, 0, &cu_fd, TESTFILE},
> +};
>
> char *TCID = "fchownat01";
> -int TST_TOTAL = TEST_CASES;
> -char pathname[256];
> -char testfile[256];
> -char testfile2[256];
> -char testfile3[256];
> -int dirfd, fd, ret;
> -int fds[TEST_CASES];
> -char *filenames[TEST_CASES];
> -int expected_errno[TEST_CASES] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
> -int flags[TEST_CASES] = { 0, 0, 0, 0, 9999, 0 };
> -
> -uid_t uid;
> -gid_t gid;
> -
> -int myfchownat(int dirfd, const char *filename, uid_t owner, gid_t group,
> - int flags)
> -{
> - return ltp_syscall(__NR_fchownat, dirfd, filename, owner, group, flags);
> -}
> +int TST_TOTAL = ARRAY_SIZE(test_cases);
> +static void fchownat_verify(const struct test_case_t *);
>
> int main(int ac, char **av)
> {
> @@ -92,153 +74,67 @@ int main(int ac, char **av)
> char *msg;
> int i;
>
> - /* Disable test if the version of the kernel is less than 2.6.16 */
> - if ((tst_kvercmp(2, 6, 16)) < 0) {
> - tst_resm(TWARN, "This test can only run on kernels that are ");
> - tst_resm(TWARN, "2.6.16 and higher");
> - exit(0);
> - }
> -
> - /***************************************************************
> - * parse standard options
> - ***************************************************************/
> if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
> tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
>
> - /***************************************************************
> - * perform global setup for test
> - ***************************************************************/
> setup();
>
> - /***************************************************************
> - * check looping state if -c option given
> - ***************************************************************/
> for (lc = 0; TEST_LOOPING(lc); lc++) {
> - setup_every_copy();
> -
> tst_count = 0;
> -
> - /*
> - * Call fchownat
> - */
> - for (i = 0; i < TST_TOTAL; i++) {
> - TEST(myfchownat
> - (fds[i], filenames[i], uid, gid, flags[i]));
> -
> - /* check return code */
> - if (TEST_ERRNO == expected_errno[i]) {
> -
> - /***************************************************************
> - * only perform functional verification if flag set (-f not given)
> - ***************************************************************/
> - if (STD_FUNCTIONAL_TEST) {
> - /* No Verification test, yet... */
> - tst_resm(TPASS,
> - "fchownat() returned the expected errno %d: %s",
> - TEST_ERRNO,
> - strerror(TEST_ERRNO));
> - }
> - } else {
> - TEST_ERROR_LOG(TEST_ERRNO);
> - tst_resm(TFAIL,
> - "fchownat() Failed, errno=%d : %s",
> - TEST_ERRNO, strerror(TEST_ERRNO));
> - }
> - }
> -
> + for (i = 0; i < TST_TOTAL; i++)
> + fchownat_verify(&test_cases[i]);
> }
>
> - /***************************************************************
> - * cleanup and exit
> - ***************************************************************/
> cleanup();
> -
> - return (0);
> + tst_exit();
> }
>
> -void setup_every_copy()
> +static void setup(void)
> {
> - /* Initialize test dir and file names */
> - sprintf(pathname, "fchownattestdir%d", getpid());
> - sprintf(testfile, "fchownattestfile%d.txt", getpid());
> - sprintf(testfile2, "/tmp/fchownattestfile%d.txt", getpid());
> - sprintf(testfile3, "fchownattestdir%d/fchownattestfile%d.txt", getpid(),
> - getpid());
> -
> - ret = mkdir(pathname, 0700);
> - if (ret < 0) {
> - perror("mkdir: ");
> - exit(-1);
> - }
> + if ((tst_kvercmp(2, 6, 16)) < 0)
> + tst_brkm(TCONF, NULL, "This test needs kernel 2.6.16 or newer");
>
> - dirfd = open(pathname, O_DIRECTORY);
> - if (dirfd < 0) {
> - perror("open: ");
> - exit(-1);
> - }
> + tst_sig(NOFORK, DEF_HANDLER, cleanup);
>
> - fd = open(testfile, O_CREAT | O_RDWR, 0600);
> - if (fd < 0) {
> - perror("open: ");
> - exit(-1);
> - }
> + TEST_PAUSE;
>
> - fd = open(testfile2, O_CREAT | O_RDWR, 0600);
> - if (fd < 0) {
> - perror("open: ");
> - exit(-1);
> - }
> + tst_tmpdir();
>
> - fd = open(testfile3, O_CREAT | O_RDWR, 0600);
> - if (fd < 0) {
> - perror("open: ");
> - exit(-1);
> - }
> + dirfd = SAFE_OPEN(cleanup, "./", O_DIRECTORY);
>
> - fds[0] = fds[1] = fds[4] = dirfd;
> - fds[2] = fd;
> - fds[3] = 100;
> - fds[5] = AT_FDCWD;
> + SAFE_TOUCH(cleanup, TESTFILE, 0600, NULL);
>
> - filenames[0] = filenames[2] = filenames[3] = filenames[4] =
> - filenames[5] = testfile;
> - filenames[1] = testfile2;
> + SAFE_TOUCH(cleanup, TESTFILE2, 0600, NULL);
> +
> + fd = SAFE_OPEN(cleanup, "testfile3", O_CREAT | O_RDWR, 0600);
> }
>
> -/***************************************************************
> - * setup() - performs all ONE TIME setup for this test.
> - ***************************************************************/
> -void setup()
> +static void fchownat_verify(const struct test_case_t *test)
> {
> - /* Set uid and gid */
> - uid = geteuid();
> - gid = getegid();
> -
> - tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> - TEST_PAUSE;
> + TEST(fchownat(*(test->fds), test->filenames, geteuid(),
> + getegid(), test->flag));
You are not checking the TEST_RETURN here as well. Given that there are
both possitive and negative testcases it should be 0 if expected errno
is 0 and -1 if expected errno is non-zero.
> + if (TEST_ERRNO == test->ret) {
Can you also please rename the test->ret to test->exp_errno?
> + tst_resm(TPASS | TTERRNO,
> + "fchownat() returned the expected errno %d: %s",
> + TEST_ERRNO, strerror(TEST_ERRNO));
> + } else {
> + TEST_ERROR_LOG(TEST_ERRNO);
> + tst_resm(TFAIL | TTERRNO,
> + "fchownat() Failed, errno=%d : %s",
> + TEST_ERRNO, strerror(TEST_ERRNO));
> + }
> }
>
> -/***************************************************************
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - * completion or premature exit.
> - ***************************************************************/
> -void cleanup()
> +static void cleanup(void)
> {
> - /* Remove them */
> - char tmppathname[256];
> - strcpy(tmppathname, pathname);
> -
> - close(fd);
> - unlink(testfile);
> - unlink(testfile2);
> - unlink(testfile3);
> - rmdir(pathname);
> -
> - /*
> - * print timing stats if that option was specified.
> - * print errno log if that option was specified.
> - */
> - TEST_CLEANUP;
> + if (fd > 0)
> + close(fd);
> +
> + if (dirfd > 0)
> + close(dirfd);
>
> + tst_rmdir();
> +
> + TEST_CLEANUP;
> }
> --
> 1.8.4.2
>
>
>
--
Cyril Hrubis
ch...@su...
|