[LTP] [PATCH 1/2] syscalls/fchmodat01: Convert to new API

[Yang Xu (Fujitsu)]

Hi Li,


On Thu, Sep 21, 2023 at 1:22 PM Yang Xu <xuyang2018.jy@fujitsu.com<mailto:xuyang2018.jy@fujitsu.com>> wrote:
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com<mailto:xuyang2018.jy@fujitsu.com>>
---
 .../kernel/syscalls/fchmodat/fchmodat01.c     | 103 +++++++-----------
 1 file changed, 42 insertions(+), 61 deletions(-)

diff --git a/testcases/kernel/syscalls/fchmodat/fchmodat01.c b/testcases/kernel/syscalls/fchmodat/fchmodat01.c
index 3deff0ebe..d9db4ec10 100644
--- a/testcases/kernel/syscalls/fchmodat/fchmodat01.c
+++ b/testcases/kernel/syscalls/fchmodat/fchmodat01.c
@@ -1,99 +1,75 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2006
- *
+ * Copyright (c) Linux Test Project, 2003-2023
  * 08/28/2006 AUTHOR: Yi Yang <yyangcdl@cn.ibm.com<mailto:yyangcdl@cn.ibm.com>>
  */

 /*\
  * [Description]
  *
- * This test case will verify basic function of fchmodat.
+ * Check the basic functionality of the faccessat() system call.

faccessat() --> fchmodat() ?

Sorry, i confused the patch I wrote earlier.
+ *
+ * - fchmodat() passes if dir_fd is file descriptor to the directory
+ *   where the file is located and pathname is relative path of the file.
+ *
+ * - fchmodat() passes if dir_fd is a bad file descriptor and pathname is
+ *   absolute path of the file.

I didn't see that the second test uses a bad dir_fd in the struct.

Yes.I will correct it.

+ *
+ * - fchmodat() passes if dir_fd is AT_FDCWD and pathname is interpreted
+ *   relative to the current working directory of the calling process.
  */

-#define _GNU_SOURCE
-
-#include <unistd.h>
-#include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include "tst_test.h"
-#include "lapi/syscalls.h"

-#ifndef AT_FDCWD
-#define AT_FDCWD -100
-#endif
+#define TESTDIR         "fchmodatdir"
+#define TESTFILE        "fchmodatfile"
+#define FILEPATH        "fchmodatdir/fchmodatfile"

-static char pathname[256];
-static char testfile[256];
-static char testfile2[256];
-static char testfile3[256];
+static int dir_fd, file_fd;
+static int atcwd_fd = AT_FDCWD;
+static char *abs_path;
+static char *test_file;
+static char *file_path;

 static struct tcase {
-       int exp_errno;
-       char *exp_errval;
+       int *fd;
+       char **filenames;
 } tcases[] = {
-       { 0, NULL},
-       { 0, NULL},
-       { ENOTDIR, "ENOTDIR"},
-       { EBADF, "EBADF"},
-       { 0, NULL},
-       { 0, NULL},
+       {&dir_fd, &test_file},
+       {&dir_fd, &abs_path},


+       {&atcwd_fd, &file_path},
 };
-static int fds[ARRAY_SIZE(tcases)];
-static char *filenames[ARRAY_SIZE(tcases)];

 static void verify_fchmodat(unsigned int i)
 {
        struct tcase *tc = &tcases[i];

-       if (tc->exp_errno == 0)
-               TST_EXP_PASS(tst_syscall(__NR_fchmodat, fds[i], filenames[i], 0600),
-                            "fchmodat() returned the expected errno %d: %s",
-                            TST_ERR, strerror(TST_ERR));
-       else
-               TST_EXP_FAIL(tst_syscall(__NR_fchmodat, fds[i], filenames[i], 0600),
-                            tc->exp_errno,
-                            "fchmodat() returned the expected errno %d: %s",
-                            TST_ERR, strerror(TST_ERR));
+       TST_EXP_PASS(fchmodat(*tc->fd, *tc->filenames, 0600, 0),
+                    "fchmodat(%d, %s, 0600, 0)",
+                    *tc->fd, *tc->filenames);


I think it would be great to verify that the permissions of a file
have been changed correctly.

OK. I will update it.

 }

 static void setup(void)
 {
-       /* Initialize test dir and file names */
-       char *abs_path = tst_get_tmpdir();
-       int p = getpid();
-
-       sprintf(pathname, "fchmodattestdir%d", p);
-       sprintf(testfile, "fchmodattest%d.txt", p);
-       sprintf(testfile2, "%s/fchmodattest%d.txt", abs_path, p);
-       sprintf(testfile3, "fchmodattestdir%d/fchmodattest%d.txt", p, p);
-
-       free(abs_path);
-
-       SAFE_MKDIR(pathname, 0700);
-
-       fds[0] = SAFE_OPEN(pathname, O_DIRECTORY);
-       fds[1] = fds[4] = fds[0];
-
-       SAFE_FILE_PRINTF(testfile, "%s", testfile);
-       SAFE_FILE_PRINTF(testfile2, "%s", testfile2);
+       char *tmpdir_path = tst_get_tmpdir();

-       fds[2] = SAFE_OPEN(testfile3, O_CREAT | O_RDWR, 0600);
-       fds[3] = 100;
-       fds[5] = AT_FDCWD;
+       abs_path = tst_aprintf("%s/%s", tmpdir_path, FILEPATH);
+       free(tmpdir_path);

-       filenames[0] = filenames[2] = filenames[3] = filenames[4] = testfile;
-       filenames[1] = testfile2;
-       filenames[5] = testfile3;
+       SAFE_MKDIR(TESTDIR, 0700);
+       dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
+       file_fd = SAFE_OPEN(FILEPATH, O_CREAT | O_RDWR, 0600);
 }

 static void cleanup(void)
 {
-       if (fds[0] > 0)
-               close(fds[0]);
-       if (fds[2] > 0)
-               close(fds[2]);
+       if (dir_fd > -1)
+               close(dir_fd);
+       if (file_fd > -1)
+               close(file_fd);

Why not use SAFE_CLOSE here?

Yes. I forgot to modify here.

 }

 static struct tst_test test = {
@@ -101,5 +77,10 @@ static struct tst_test test = {
        .test = verify_fchmodat,
        .setup = setup,
        .cleanup = cleanup,
+       .bufs = (struct tst_buffers []) {
+               {&test_file, .str = TESTFILE},
+               {&file_path, .str = FILEPATH},
+               {},
+       },
        .needs_tmpdir = 1,
 };

The rest part looks good.



Thanks for your patient review.

Best Regards

Yang Xu

--
Regards,
Li Wang