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

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 .../kernel/syscalls/fchmodat/fchmodat01.c     | 103 +++++++-----------
 1 file changed, 42 insertions(+), 61 deletions(-)

HI



Ping.



Best Regards

Yang Xu


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.
+ *
+ * - 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.
+ *
+ * - 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);
 }

 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);
 }

 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,
 };

On Thu, Sep 21, 2023 at 1:22 PM Yang Xu <xuyang2018.jy@fujitsu.com> wrote:

> Signed-off-by: Yang Xu <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>
>   */
>
>  /*\
>   * [Description]
>   *
> - * This test case will verify basic function of fchmodat.
> + * Check the basic functionality of the faccessat() system call.
>

faccessat() --> fchmodat() ?



> + *
> + * - 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.



> + *
> + * - 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.



>  }
>
>  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?



>  }
>
>  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.

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