* [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API
@ 2021-09-29 8:32 zhanglianjie
2021-09-29 8:32 ` [LTP] [PATCH 2/2] syscalls/readdir21: " zhanglianjie
2021-10-07 11:35 ` [LTP] [PATCH 1/2] syscalls/readdir01: " Cyril Hrubis
0 siblings, 2 replies; 7+ messages in thread
From: zhanglianjie @ 2021-09-29 8:32 UTC (permalink / raw)
To: ltp
Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
diff --git a/testcases/kernel/syscalls/readdir/readdir01.c b/testcases/kernel/syscalls/readdir/readdir01.c
index 15d575b80..86d2bd51e 100644
--- a/testcases/kernel/syscalls/readdir/readdir01.c
+++ b/testcases/kernel/syscalls/readdir/readdir01.c
@@ -1,25 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved.
*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * 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.
- *
- * Further, this software is distributed without any warranty that it is
- * free of the rightful claim of any third person regarding infringement
- * or the like. Any license provided herein, whether implied or
- * otherwise, applies only to this software file. Patent licenses, if
- * any, provided herein do not apply to combinations of this program with
- * other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
* Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
* Mountain View, CA 94043, or:
*
@@ -30,287 +12,62 @@
* http://oss.sgi.com/projects/GenInfo/NoticeExplan/
*
*/
-/* $Id: readdir01.c,v 1.7 2009/03/23 13:36:01 subrata_modak Exp $ */
-/**********************************************************
- *
- * OS Test - Silicon Graphics, Inc.
- *
- * TEST IDENTIFIER : readdir01
- *
- * EXECUTED BY : anyone
- *
- * TEST TITLE : write multiple files and try to find them with readdir
- *
- * TEST CASE TOTAL :
- *
- * WALL CLOCK TIME :
- *
- * CPU TYPES : ALL
- *
- * AUTHOR : Nate Straz
- *
- * CO-PILOT :
- *
- * DATE STARTED : 02/16/2001
- *
- * INITIAL RELEASE : Linux 2.4.x
- *
- * TEST CASES
- *
- * 1.) Create n files and check that readdir() finds n files
- *
- * INPUT SPECIFICATIONS
- * The standard options for system call tests are accepted.
- * (See the parse_opts(3) man page).
- *
- * OUTPUT SPECIFICATIONS
- *$
- * DURATION
- * Terminates - with frequency and infinite modes.
- *
- * SIGNALS
- * Uses SIGUSR1 to pause before test if option set.
- * (See the parse_opts(3) man page).
- *
- * RESOURCES
- * None
- *
- * ENVIRONMENTAL NEEDS
- * No run-time environmental needs.
- *
- * SPECIAL PROCEDURAL REQUIREMENTS
- * None
- *
- * INTERCASE DEPENDENCIES
- * None
- *
- * DETAILED DESCRIPTION
- * This is a Phase I test for the readdir(2) system call. It is intended
- * to provide a limited exposure of the system call, for now. It
- * should/will be extended when full functional tests are written for
- * readdir(2).
- *
- * Setup:
- * Setup signal handling.
- * Pause for SIGUSR1 if option specified.
- *
- * Test:
- * Loop if the proper options are given.
- * Execute system call
- * Check return code, if system call failed (return=-1)
- * Log the errno and Issue a FAIL message.
- * Otherwise, Issue a PASS message.
- *
- * Cleanup:
- * Print errno log and/or timing stats if options given
- *
- *
- *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
-
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <dirent.h>
-#include <unistd.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-#include "test.h"
-
- /* The setup and cleanup functions are basic parts of a test case. These
- * steps are usually put in separate functions for clarity. The help function
- * is only needed when you are adding new command line options.
- */
-void setup();
-void help();
-void cleanup();
-
-char *TCID = "readdir01";
-int TST_TOTAL = 2;
-
-#define BASENAME "readdirfile"
-char Basename[255];
-char Fname[255];
-int Nfiles = 0;
+/*\
+ * [Description]
+ *
+ * The test for the readdir(2) system call.
+ * Create n files and check that readdir() finds n files
+ */
+#include <stdio.h>
+#include "tst_test.h"
-char *Nfilearg;
-int Nflag = 0;
+char prefix[32];
+char fname[255];
+int nfiles = 10;
-option_t options[] = {
- {"N:", &Nflag, &Nfilearg}, /* -N #files */
- {NULL, NULL, NULL}
-};
+static void setup(void)
+{
+ sprintf(prefix, "%s_%d.", "readdirfile", getpid());
+}
-/***********************************************************************
- * Main
- ***********************************************************************/
-int main(int ac, char **av)
+static void verify_readdir(void)
{
- int lc;
- int cnt;
- int nfiles, fd;
+ int i;
+ int fd;
+ int cnt = 0;
char fname[255];
DIR *test_dir;
struct dirent *dptr;
- tst_parse_opts(ac, av, options, &help);
-
- if (Nflag) {
- if (sscanf(Nfilearg, "%i", &Nfiles) != 1) {
- tst_brkm(TBROK, NULL, "--N option arg is not a number");
- }
+ for (i = 0; i < nfiles; i++) {
+ sprintf(fname, "%s_%d", prefix, i);
+ fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0700);
+ SAFE_WRITE(1, fd, "hello\n", 6);
+ SAFE_CLOSE(fd);
}
- /***************************************************************
- * perform global setup for test
- ***************************************************************/
- /* Next you should run a setup routine to make sure your environment is
- * sane.
- */
- setup();
-
- /***************************************************************
- * check looping state
- ***************************************************************/
- /* TEST_LOOPING() is a macro that will make sure the test continues
- * looping according to the standard command line args.
- */
- for (lc = 0; TEST_LOOPING(lc); lc++) {
-
- tst_count = 0;
-
- if (Nfiles)
- nfiles = Nfiles;
- else
- /* min of 10 links and max of a 100 links */
- nfiles = (lc % 90) + 10;
-
- /* create a bunch of files to look at */
- for (cnt = 0; cnt < nfiles; cnt++) {
-
- sprintf(fname, "%s%d", Basename, cnt);
- if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
- tst_brkm(TBROK, cleanup,
- "open(%s, O_RDWR|O_CREAT,0700) Failed, errno=%d : %s",
- fname, errno, strerror(errno));
- } else if (write(fd, "hello\n", 6) < 0) {
- tst_brkm(TBROK, cleanup,
- "write(%s, \"hello\\n\", 6) Failed, errno=%d : %s",
- fname, errno, strerror(errno));
- } else if (close(fd) < 0) {
- tst_resm(TWARN,
- "close(%s) Failed, errno=%d : %s",
- fname, errno, strerror(errno));
- }
- }
-
- if ((test_dir = opendir(".")) == NULL) {
- tst_resm(TFAIL, "opendir(\".\") Failed, errno=%d : %s",
- errno, strerror(errno));
- } else {
- /* count the entries we find to see if any are missing */
- cnt = 0;
- errno = 0;
- while ((dptr = readdir(test_dir)) != 0) {
- if (strcmp(dptr->d_name, ".")
- && strcmp(dptr->d_name, ".."))
- cnt++;
- }
-
- if (errno != 0) {
- tst_resm(TFAIL,
- "readir(test_dir) Failed on try %d, errno=%d : %s",
- cnt + 1, errno, strerror(errno));
- }
- if (cnt == nfiles) {
- tst_resm(TPASS,
- "found all %d that were created",
- nfiles);
- } else if (cnt > nfiles) {
- tst_resm(TFAIL,
- "found more files than were created");
- tst_resm(TINFO, "created: %d, found: %d",
- nfiles, cnt);
- } else {
- tst_resm(TFAIL,
- "found less files than were created");
- tst_resm(TINFO, "created: %d, found: %d",
- nfiles, cnt);
- }
- }
-
- /* Here we clean up after the test case so we can do another iteration.
- */
- for (cnt = 0; cnt < nfiles; cnt++) {
-
- sprintf(fname, "%s%d", Basename, cnt);
-
- if (unlink(fname) == -1) {
- tst_resm(TWARN,
- "unlink(%s) Failed, errno=%d : %s",
- Fname, errno, strerror(errno));
- }
- }
-
+ test_dir = SAFE_OPENDIR(".");
+ while ((dptr = SAFE_READDIR(test_dir)) != 0) {
+ if (strcmp(dptr->d_name, ".")
+ && strcmp(dptr->d_name, ".."))
+ cnt++;
}
- /***************************************************************
- * cleanup and exit
- ***************************************************************/
- cleanup();
-
- tst_exit();
-}
-
-/***************************************************************
- * help
- ***************************************************************/
-/* The custom help() function is really simple. Just write your help message to
- * standard out. Your help function will be called after the standard options
- * have been printed
- */
-void help(void)
-{
- printf(" -N #files : create #files files every iteration\n");
-}
-
-/***************************************************************
- * setup() - performs all ONE TIME setup for this test.
- ***************************************************************/
-void setup(void)
-{
- /* You will want to enable some signal handling so you can capture
- * unexpected signals like SIGSEGV.
- */
- tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
- /* One cavet that hasn't been fixed yet. TEST_PAUSE contains the code to
- * fork the test with the -c option. You want to make sure you do this
- * before you create your temporary directory.
- */
- TEST_PAUSE;
-
- /* If you are doing any file work, you should use a temporary directory. We
- * provide tst_tmpdir() which will create a uniquely named temporary
- * directory and cd into it. You can now create files in the current
- * directory without worrying.
- */
- tst_tmpdir();
-
- sprintf(Basename, "%s_%d.", BASENAME, getpid());
+ if (cnt == nfiles) {
+ tst_res(TPASS,
+ "found all %d that were created",
+ nfiles);
+ } else {
+ tst_res(TFAIL,
+ "found %s files than were created, created: %d, found: %d",
+ cnt > nfiles ? "more" : "less", nfiles, cnt);
+ }
}
-/***************************************************************
- * cleanup() - performs all ONE TIME cleanup for this test at
- * completion or premature exit.
- ***************************************************************/
-void cleanup(void)
-{
-
- /* If you use a temporary directory, you need to be sure you remove it. Use
- * tst_rmdir() to do it automatically.$
- */
- tst_rmdir();
+static struct tst_test test = {
+ .setup = setup,
+ .test_all = verify_readdir,
+ .needs_tmpdir = 1,
+};
-}
--
2.20.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [LTP] [PATCH 2/2] syscalls/readdir21: Convert to new API
2021-09-29 8:32 [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API zhanglianjie
@ 2021-09-29 8:32 ` zhanglianjie
2021-10-07 11:54 ` Cyril Hrubis
2021-10-07 11:35 ` [LTP] [PATCH 1/2] syscalls/readdir01: " Cyril Hrubis
1 sibling, 1 reply; 7+ messages in thread
From: zhanglianjie @ 2021-09-29 8:32 UTC (permalink / raw)
To: ltp
Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
diff --git a/testcases/kernel/syscalls/readdir/readdir21.c b/testcases/kernel/syscalls/readdir/readdir21.c
index 205e0715b..1bc3cc199 100644
--- a/testcases/kernel/syscalls/readdir/readdir21.c
+++ b/testcases/kernel/syscalls/readdir/readdir21.c
@@ -1,20 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2014 Fujitsu Ltd.
* Author: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
-/*
+
+/*\
+ * [Description]
+ *
* Test Description:
* Verify that,
* 1. Creat a directory and open it, then delete the directory, ENOENT would
@@ -23,28 +15,16 @@
* 3. Invalid file descriptor fd, EBADF would return.
* 4. Argument points outside the calling process's address space, EFAULT
* would return.
- *
- * PS:
- * This file is for readdir(2) and the other files(readdir01.c and
- * readdir02.c) are for readdir(3).
*/
-#define _GNU_SOURCE
+//#define _GNU_SOURCE
#include <stdio.h>
-#include <sys/types.h>
#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <unistd.h>
-#include <sys/mman.h>
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
#include "lapi/syscalls.h"
#include "lapi/readdir.h"
-char *TCID = "readdir21";
-
#define TEST_DIR "test_dir"
#define TEST_DIR4 "test_dir4"
#define TEST_FILE "test_file"
@@ -55,96 +35,46 @@ static unsigned int del_dir_fd, file_fd;
static unsigned int invalid_fd = 999;
static unsigned int dir_fd;
static struct old_linux_dirent dirp;
-static void setup(void);
-static void cleanup(void);
-static struct test_case_t {
+static struct tcase {
unsigned int *fd;
struct old_linux_dirent *dirp;
unsigned int count;
int exp_errno;
-} test_cases[] = {
+} tcases[] = {
{&del_dir_fd, &dirp, sizeof(struct old_linux_dirent), ENOENT},
{&file_fd, &dirp, sizeof(struct old_linux_dirent), ENOTDIR},
{&invalid_fd, &dirp, sizeof(struct old_linux_dirent), EBADF},
-#if !defined(UCLINUX)
{&dir_fd, (struct old_linux_dirent *)-1,
sizeof(struct old_linux_dirent), EFAULT},
-#endif
};
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-static void readdir_verify(const struct test_case_t *);
-
-int main(int argc, char **argv)
-{
- int i, lc;
-
- tst_parse_opts(argc, argv, NULL, NULL);
-
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); lc++) {
- tst_count = 0;
- for (i = 0; i < TST_TOTAL; i++)
- readdir_verify(&test_cases[i]);
- }
-
- cleanup();
- tst_exit();
-}
-
static void setup(void)
{
- tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
- TEST_PAUSE;
+ SAFE_MKDIR(TEST_DIR, DIR_MODE);
+ del_dir_fd = SAFE_OPEN(TEST_DIR, O_RDONLY | O_DIRECTORY);
+ SAFE_RMDIR(TEST_DIR);
- tst_tmpdir();
+ file_fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0777);
- SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
- del_dir_fd = SAFE_OPEN(cleanup, TEST_DIR, O_RDONLY | O_DIRECTORY);
- SAFE_RMDIR(cleanup, TEST_DIR);
+ SAFE_MKDIR(TEST_DIR4, DIR_MODE);
+ dir_fd = SAFE_OPEN(TEST_DIR4, O_RDONLY | O_DIRECTORY);
- file_fd = SAFE_OPEN(cleanup, TEST_FILE, O_RDWR | O_CREAT, 0777);
-
- SAFE_MKDIR(cleanup, TEST_DIR4, DIR_MODE);
- dir_fd = SAFE_OPEN(cleanup, TEST_DIR4, O_RDONLY | O_DIRECTORY);
-
-#if !defined(UCLINUX)
- test_cases[3].dirp = SAFE_MMAP(cleanup, 0, 1, PROT_NONE,
- MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-#endif
+ tcases[3].dirp = tst_get_bad_addr(NULL);
}
-static void readdir_verify(const struct test_case_t *test)
+static void verify_readdir(unsigned int nr)
{
- TEST(ltp_syscall(__NR_readdir, *test->fd, test->dirp, test->count));
-
- if (TEST_RETURN != -1) {
- tst_resm(TFAIL, "readdir() succeeded unexpectedly");
- return;
- }
+ struct tcase *tc = &tcases[nr];
- if (TEST_ERRNO == test->exp_errno) {
- tst_resm(TPASS | TTERRNO, "readdir() failed as expected");
- } else {
- tst_resm(TFAIL | TTERRNO,
- "readdir() failed unexpectedly; expected: %d - %s",
- test->exp_errno, strerror(test->exp_errno));
- }
+ TST_EXP_FAIL(tst_syscall(__NR_readdir, *tc->fd, tc->dirp, tc->count),
+ tc->exp_errno, "readdir() failed unexpectedly, expected: %d - %s",
+ tc->exp_errno, strerror(tc->exp_errno));
}
-static void cleanup(void)
-{
- if (dir_fd > 0)
- close(dir_fd);
-
- if (del_dir_fd > 0)
- close(del_dir_fd);
-
- if (file_fd > 0)
- close(file_fd);
-
- tst_rmdir();
-}
+static struct tst_test test = {
+ .tcnt = ARRAY_SIZE(tcases),
+ .setup = setup,
+ .test = verify_readdir,
+ .needs_tmpdir = 1,
+};
--
2.20.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH 2/2] syscalls/readdir21: Convert to new API
2021-09-29 8:32 ` [LTP] [PATCH 2/2] syscalls/readdir21: " zhanglianjie
@ 2021-10-07 11:54 ` Cyril Hrubis
2021-10-08 12:42 ` zhanglianjie
0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-10-07 11:54 UTC (permalink / raw)
To: zhanglianjie; +Cc: ltp
On Wed, Sep 29, 2021 at 04:32:49PM +0800, zhanglianjie wrote:
> Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
>
> diff --git a/testcases/kernel/syscalls/readdir/readdir21.c b/testcases/kernel/syscalls/readdir/readdir21.c
> index 205e0715b..1bc3cc199 100644
> --- a/testcases/kernel/syscalls/readdir/readdir21.c
> +++ b/testcases/kernel/syscalls/readdir/readdir21.c
> @@ -1,20 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (c) 2014 Fujitsu Ltd.
> * Author: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * 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.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write the Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> */
> -/*
> +
> +/*\
> + * [Description]
> + *
> * Test Description:
> * Verify that,
> * 1. Creat a directory and open it, then delete the directory, ENOENT would
> @@ -23,28 +15,16 @@
> * 3. Invalid file descriptor fd, EBADF would return.
> * 4. Argument points outside the calling process's address space, EFAULT
> * would return.
This should be reformatted so that it renders nicely in asccidoc as:
/*\
* [Description]
*
* Verify that readdir will fail with:
*
* - ENOENT when passed a fd to a deleted directory
* - ENOTDIR when passed fd that does not point to a directory
* - EBADFD when passed an invalid fd
* - EFAULT when passed invalid buffer pointer
*/
> - *
> - * PS:
> - * This file is for readdir(2) and the other files(readdir01.c and
> - * readdir02.c) are for readdir(3).
> */
>
> -#define _GNU_SOURCE
> +//#define _GNU_SOURCE
Just remove it if it's not needed.
> #include <stdio.h>
> -#include <sys/types.h>
> #include <sys/stat.h>
> -#include <fcntl.h>
> -#include <errno.h>
> -#include <unistd.h>
> -#include <sys/mman.h>
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
> #include "lapi/syscalls.h"
> #include "lapi/readdir.h"
>
> -char *TCID = "readdir21";
> -
> #define TEST_DIR "test_dir"
> #define TEST_DIR4 "test_dir4"
> #define TEST_FILE "test_file"
> @@ -55,96 +35,46 @@ static unsigned int del_dir_fd, file_fd;
> static unsigned int invalid_fd = 999;
> static unsigned int dir_fd;
> static struct old_linux_dirent dirp;
> -static void setup(void);
> -static void cleanup(void);
>
> -static struct test_case_t {
> +static struct tcase {
> unsigned int *fd;
> struct old_linux_dirent *dirp;
> unsigned int count;
> int exp_errno;
> -} test_cases[] = {
> +} tcases[] = {
> {&del_dir_fd, &dirp, sizeof(struct old_linux_dirent), ENOENT},
> {&file_fd, &dirp, sizeof(struct old_linux_dirent), ENOTDIR},
> {&invalid_fd, &dirp, sizeof(struct old_linux_dirent), EBADF},
> -#if !defined(UCLINUX)
> {&dir_fd, (struct old_linux_dirent *)-1,
> sizeof(struct old_linux_dirent), EFAULT},
> -#endif
> };
I guess that we also miss EINVAL error here with invalid count.
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> -static void readdir_verify(const struct test_case_t *);
> -
> -int main(int argc, char **argv)
> -{
> - int i, lc;
> -
> - tst_parse_opts(argc, argv, NULL, NULL);
> -
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> - tst_count = 0;
> - for (i = 0; i < TST_TOTAL; i++)
> - readdir_verify(&test_cases[i]);
> - }
> -
> - cleanup();
> - tst_exit();
> -}
> -
> static void setup(void)
> {
> - tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> - TEST_PAUSE;
> + SAFE_MKDIR(TEST_DIR, DIR_MODE);
> + del_dir_fd = SAFE_OPEN(TEST_DIR, O_RDONLY | O_DIRECTORY);
> + SAFE_RMDIR(TEST_DIR);
>
> - tst_tmpdir();
> + file_fd = SAFE_OPEN(TEST_FILE, O_RDWR | O_CREAT, 0777);
>
> - SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
> - del_dir_fd = SAFE_OPEN(cleanup, TEST_DIR, O_RDONLY | O_DIRECTORY);
> - SAFE_RMDIR(cleanup, TEST_DIR);
> + SAFE_MKDIR(TEST_DIR4, DIR_MODE);
> + dir_fd = SAFE_OPEN(TEST_DIR4, O_RDONLY | O_DIRECTORY);
>
> - file_fd = SAFE_OPEN(cleanup, TEST_FILE, O_RDWR | O_CREAT, 0777);
> -
> - SAFE_MKDIR(cleanup, TEST_DIR4, DIR_MODE);
> - dir_fd = SAFE_OPEN(cleanup, TEST_DIR4, O_RDONLY | O_DIRECTORY);
> -
> -#if !defined(UCLINUX)
> - test_cases[3].dirp = SAFE_MMAP(cleanup, 0, 1, PROT_NONE,
> - MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> -#endif
> + tcases[3].dirp = tst_get_bad_addr(NULL);
We shouldn't rely on the tcases structure order. We usually solve this
with a for loop in the setup that looks for EFAULT errno as with:
for (i = 0; i < ARRAY_SIZE(tcases); i++) {
if (tcases[i].exp_errno == EBADFD)
tcases[i].dirp = tst_get_bad_addr(NULL);
}
> }
>
> -static void readdir_verify(const struct test_case_t *test)
> +static void verify_readdir(unsigned int nr)
> {
> - TEST(ltp_syscall(__NR_readdir, *test->fd, test->dirp, test->count));
> -
> - if (TEST_RETURN != -1) {
> - tst_resm(TFAIL, "readdir() succeeded unexpectedly");
> - return;
> - }
> + struct tcase *tc = &tcases[nr];
>
> - if (TEST_ERRNO == test->exp_errno) {
> - tst_resm(TPASS | TTERRNO, "readdir() failed as expected");
> - } else {
> - tst_resm(TFAIL | TTERRNO,
> - "readdir() failed unexpectedly; expected: %d - %s",
> - test->exp_errno, strerror(test->exp_errno));
> - }
> + TST_EXP_FAIL(tst_syscall(__NR_readdir, *tc->fd, tc->dirp, tc->count),
> + tc->exp_errno, "readdir() failed unexpectedly, expected: %d - %s",
> + tc->exp_errno, strerror(tc->exp_errno));
This produces a very confusing messages. The TST_EXP_*() macros should
print what is being tested, since the part that describes the result is
appended automatically.
So here we should probably add a short description to the tcases
structure and print it with the macro as:
...
} tcases[] = {
{&del_dir_fd, &dirp, sizeof(struct old_linux_dirent), ENOENT, "directory deleted"},
...
}
...
TST_EXP_FAIL(tst_syscall(__NR_readdir, *tc->fd, tc->dirp, tc->count),
tc->exp_errno, "readdir() with %s", tc->desc);
Which would produce:
readdir21.c:70: TPASS: readdir() with directory deleted : ENOENT (2)
Instead of the confusing:
readdir21.c:70: TPASS: readdir() failed unexpectedly, expected: 2 - No such file or directory : ENOENT (2)
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH 2/2] syscalls/readdir21: Convert to new API
2021-10-07 11:54 ` Cyril Hrubis
@ 2021-10-08 12:42 ` zhanglianjie
2021-10-08 14:43 ` Cyril Hrubis
0 siblings, 1 reply; 7+ messages in thread
From: zhanglianjie @ 2021-10-08 12:42 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hi,
On 2021-10-07 19:54, Cyril Hrubis wrote:
> On Wed, Sep 29, 2021 at 04:32:49PM +0800, zhanglianjie wrote:
>> Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
>> -static struct test_case_t {
>> +static struct tcase {
>> unsigned int *fd;
>> struct old_linux_dirent *dirp;
>> unsigned int count;
>> int exp_errno;
>> -} test_cases[] = {
>> +} tcases[] = {
>> {&del_dir_fd, &dirp, sizeof(struct old_linux_dirent), ENOENT},
>> {&file_fd, &dirp, sizeof(struct old_linux_dirent), ENOTDIR},
>> {&invalid_fd, &dirp, sizeof(struct old_linux_dirent), EBADF},
>> -#if !defined(UCLINUX)
>> {&dir_fd, (struct old_linux_dirent *)-1,
>> sizeof(struct old_linux_dirent), EFAULT},
>> -#endif
>> };
>
> I guess that we also miss EINVAL error here with invalid count.
>
The count here is not used, there is no way to test EINVAL.
--
Regards,
Zhang Lianjie
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API
2021-09-29 8:32 [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API zhanglianjie
2021-09-29 8:32 ` [LTP] [PATCH 2/2] syscalls/readdir21: " zhanglianjie
@ 2021-10-07 11:35 ` Cyril Hrubis
2021-10-08 12:37 ` zhanglianjie
1 sibling, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-10-07 11:35 UTC (permalink / raw)
To: zhanglianjie; +Cc: ltp
Hi!
> +static void setup(void)
> +{
> + sprintf(prefix, "%s_%d.", "readdirfile", getpid());
Since the test runs in it's own temporary directory there is no need to
prefix everything with the pid.
> +}
>
> -/***********************************************************************
> - * Main
> - ***********************************************************************/
> -int main(int ac, char **av)
> +static void verify_readdir(void)
> {
> - int lc;
> - int cnt;
> - int nfiles, fd;
> + int i;
> + int fd;
> + int cnt = 0;
> char fname[255];
> DIR *test_dir;
> struct dirent *dptr;
>
> - tst_parse_opts(ac, av, options, &help);
> -
> - if (Nflag) {
> - if (sscanf(Nfilearg, "%i", &Nfiles) != 1) {
> - tst_brkm(TBROK, NULL, "--N option arg is not a number");
> - }
> + for (i = 0; i < nfiles; i++) {
> + sprintf(fname, "%s_%d", prefix, i);
> + fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0700);
> + SAFE_WRITE(1, fd, "hello\n", 6);
> + SAFE_CLOSE(fd);
> }
This loop could be moved to the test setup, there is no need to
re-create the files on each iteration.
>
> - /***************************************************************
> - * perform global setup for test
> - ***************************************************************/
> - /* Next you should run a setup routine to make sure your environment is
> - * sane.
> - */
> - setup();
> -
> - /***************************************************************
> - * check looping state
> - ***************************************************************/
> - /* TEST_LOOPING() is a macro that will make sure the test continues
> - * looping according to the standard command line args.
> - */
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> - tst_count = 0;
> -
> - if (Nfiles)
> - nfiles = Nfiles;
> - else
> - /* min of 10 links and max of a 100 links */
> - nfiles = (lc % 90) + 10;
> -
> - /* create a bunch of files to look at */
> - for (cnt = 0; cnt < nfiles; cnt++) {
> -
> - sprintf(fname, "%s%d", Basename, cnt);
> - if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
> - tst_brkm(TBROK, cleanup,
> - "open(%s, O_RDWR|O_CREAT,0700) Failed, errno=%d : %s",
> - fname, errno, strerror(errno));
> - } else if (write(fd, "hello\n", 6) < 0) {
> - tst_brkm(TBROK, cleanup,
> - "write(%s, \"hello\\n\", 6) Failed, errno=%d : %s",
> - fname, errno, strerror(errno));
> - } else if (close(fd) < 0) {
> - tst_resm(TWARN,
> - "close(%s) Failed, errno=%d : %s",
> - fname, errno, strerror(errno));
> - }
> - }
> -
> - if ((test_dir = opendir(".")) == NULL) {
> - tst_resm(TFAIL, "opendir(\".\") Failed, errno=%d : %s",
> - errno, strerror(errno));
> - } else {
> - /* count the entries we find to see if any are missing */
> - cnt = 0;
> - errno = 0;
> - while ((dptr = readdir(test_dir)) != 0) {
> - if (strcmp(dptr->d_name, ".")
> - && strcmp(dptr->d_name, ".."))
> - cnt++;
> - }
> -
> - if (errno != 0) {
> - tst_resm(TFAIL,
> - "readir(test_dir) Failed on try %d, errno=%d : %s",
> - cnt + 1, errno, strerror(errno));
> - }
> - if (cnt == nfiles) {
> - tst_resm(TPASS,
> - "found all %d that were created",
> - nfiles);
> - } else if (cnt > nfiles) {
> - tst_resm(TFAIL,
> - "found more files than were created");
> - tst_resm(TINFO, "created: %d, found: %d",
> - nfiles, cnt);
> - } else {
> - tst_resm(TFAIL,
> - "found less files than were created");
> - tst_resm(TINFO, "created: %d, found: %d",
> - nfiles, cnt);
> - }
> - }
> -
> - /* Here we clean up after the test case so we can do another iteration.
> - */
> - for (cnt = 0; cnt < nfiles; cnt++) {
> -
> - sprintf(fname, "%s%d", Basename, cnt);
> -
> - if (unlink(fname) == -1) {
> - tst_resm(TWARN,
> - "unlink(%s) Failed, errno=%d : %s",
> - Fname, errno, strerror(errno));
> - }
> - }
> -
> + test_dir = SAFE_OPENDIR(".");
> + while ((dptr = SAFE_READDIR(test_dir)) != 0) {
> + if (strcmp(dptr->d_name, ".")
> + && strcmp(dptr->d_name, ".."))
> + cnt++;
> }
I would have probably written this as:
while ((ent = SAFE_READDIR(test_dir))) {
if (!strcmp(ent->d_name, "." || !strcmp(ent->d_name, ".")
continue;
cnt++;
}
Also I guess that we can check that the filename is filled correctly as
well, it has to start with prefix at least.
> - /***************************************************************
> - * cleanup and exit
> - ***************************************************************/
> - cleanup();
> -
> - tst_exit();
> -}
> -
> -/***************************************************************
> - * help
> - ***************************************************************/
> -/* The custom help() function is really simple. Just write your help message to
> - * standard out. Your help function will be called after the standard options
> - * have been printed
> - */
> -void help(void)
> -{
> - printf(" -N #files : create #files files every iteration\n");
> -}
> -
> -/***************************************************************
> - * setup() - performs all ONE TIME setup for this test.
> - ***************************************************************/
> -void setup(void)
> -{
> - /* You will want to enable some signal handling so you can capture
> - * unexpected signals like SIGSEGV.
> - */
> - tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> - /* One cavet that hasn't been fixed yet. TEST_PAUSE contains the code to
> - * fork the test with the -c option. You want to make sure you do this
> - * before you create your temporary directory.
> - */
> - TEST_PAUSE;
> -
> - /* If you are doing any file work, you should use a temporary directory. We
> - * provide tst_tmpdir() which will create a uniquely named temporary
> - * directory and cd into it. You can now create files in the current
> - * directory without worrying.
> - */
> - tst_tmpdir();
> -
> - sprintf(Basename, "%s_%d.", BASENAME, getpid());
> + if (cnt == nfiles) {
> + tst_res(TPASS,
> + "found all %d that were created",
> + nfiles);
> + } else {
> + tst_res(TFAIL,
> + "found %s files than were created, created: %d, found: %d",
> + cnt > nfiles ? "more" : "less", nfiles, cnt);
> + }
Why the newline after TPASS, TFAIL, ? We indent the format string as if
it continued after the TFAIL/TPASS anyways.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API
2021-10-07 11:35 ` [LTP] [PATCH 1/2] syscalls/readdir01: " Cyril Hrubis
@ 2021-10-08 12:37 ` zhanglianjie
0 siblings, 0 replies; 7+ messages in thread
From: zhanglianjie @ 2021-10-08 12:37 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hi,
I will resubmit after revision, thank you for your review.
On 2021-10-07 19:35, Cyril Hrubis wrote:
> Hi!
>> +static void setup(void)
>> +{
>> + sprintf(prefix, "%s_%d.", "readdirfile", getpid());
>
> Since the test runs in it's own temporary directory there is no need to
> prefix everything with the pid.
>
>> +}
>>
>> -/***********************************************************************
>> - * Main
>> - ***********************************************************************/
>> -int main(int ac, char **av)
>> +static void verify_readdir(void)
>> {
>> - int lc;
>> - int cnt;
>> - int nfiles, fd;
>> + int i;
>> + int fd;
>> + int cnt = 0;
>> char fname[255];
>> DIR *test_dir;
>> struct dirent *dptr;
>>
>> - tst_parse_opts(ac, av, options, &help);
>> -
>> - if (Nflag) {
>> - if (sscanf(Nfilearg, "%i", &Nfiles) != 1) {
>> - tst_brkm(TBROK, NULL, "--N option arg is not a number");
>> - }
>> + for (i = 0; i < nfiles; i++) {
>> + sprintf(fname, "%s_%d", prefix, i);
>> + fd = SAFE_OPEN(fname, O_RDWR | O_CREAT, 0700);
>> + SAFE_WRITE(1, fd, "hello\n", 6);
>> + SAFE_CLOSE(fd);
>> }
>
> This loop could be moved to the test setup, there is no need to
> re-create the files on each iteration.
>
>>
>> - /***************************************************************
>> - * perform global setup for test
>> - ***************************************************************/
>> - /* Next you should run a setup routine to make sure your environment is
>> - * sane.
>> - */
>> - setup();
>> -
>> - /***************************************************************
>> - * check looping state
>> - ***************************************************************/
>> - /* TEST_LOOPING() is a macro that will make sure the test continues
>> - * looping according to the standard command line args.
>> - */
>> - for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -
>> - tst_count = 0;
>> -
>> - if (Nfiles)
>> - nfiles = Nfiles;
>> - else
>> - /* min of 10 links and max of a 100 links */
>> - nfiles = (lc % 90) + 10;
>> -
>> - /* create a bunch of files to look at */
>> - for (cnt = 0; cnt < nfiles; cnt++) {
>> -
>> - sprintf(fname, "%s%d", Basename, cnt);
>> - if ((fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1) {
>> - tst_brkm(TBROK, cleanup,
>> - "open(%s, O_RDWR|O_CREAT,0700) Failed, errno=%d : %s",
>> - fname, errno, strerror(errno));
>> - } else if (write(fd, "hello\n", 6) < 0) {
>> - tst_brkm(TBROK, cleanup,
>> - "write(%s, \"hello\\n\", 6) Failed, errno=%d : %s",
>> - fname, errno, strerror(errno));
>> - } else if (close(fd) < 0) {
>> - tst_resm(TWARN,
>> - "close(%s) Failed, errno=%d : %s",
>> - fname, errno, strerror(errno));
>> - }
>> - }
>> -
>> - if ((test_dir = opendir(".")) == NULL) {
>> - tst_resm(TFAIL, "opendir(\".\") Failed, errno=%d : %s",
>> - errno, strerror(errno));
>> - } else {
>> - /* count the entries we find to see if any are missing */
>> - cnt = 0;
>> - errno = 0;
>> - while ((dptr = readdir(test_dir)) != 0) {
>> - if (strcmp(dptr->d_name, ".")
>> - && strcmp(dptr->d_name, ".."))
>> - cnt++;
>> - }
>> -
>> - if (errno != 0) {
>> - tst_resm(TFAIL,
>> - "readir(test_dir) Failed on try %d, errno=%d : %s",
>> - cnt + 1, errno, strerror(errno));
>> - }
>> - if (cnt == nfiles) {
>> - tst_resm(TPASS,
>> - "found all %d that were created",
>> - nfiles);
>> - } else if (cnt > nfiles) {
>> - tst_resm(TFAIL,
>> - "found more files than were created");
>> - tst_resm(TINFO, "created: %d, found: %d",
>> - nfiles, cnt);
>> - } else {
>> - tst_resm(TFAIL,
>> - "found less files than were created");
>> - tst_resm(TINFO, "created: %d, found: %d",
>> - nfiles, cnt);
>> - }
>> - }
>> -
>> - /* Here we clean up after the test case so we can do another iteration.
>> - */
>> - for (cnt = 0; cnt < nfiles; cnt++) {
>> -
>> - sprintf(fname, "%s%d", Basename, cnt);
>> -
>> - if (unlink(fname) == -1) {
>> - tst_resm(TWARN,
>> - "unlink(%s) Failed, errno=%d : %s",
>> - Fname, errno, strerror(errno));
>> - }
>> - }
>> -
>> + test_dir = SAFE_OPENDIR(".");
>> + while ((dptr = SAFE_READDIR(test_dir)) != 0) {
>> + if (strcmp(dptr->d_name, ".")
>> + && strcmp(dptr->d_name, ".."))
>> + cnt++;
>> }
>
> I would have probably written this as:
>
> while ((ent = SAFE_READDIR(test_dir))) {
> if (!strcmp(ent->d_name, "." || !strcmp(ent->d_name, ".")
> continue;
>
> cnt++;
> }
>
> Also I guess that we can check that the filename is filled correctly as
> well, it has to start with prefix at least.
>
>> - /***************************************************************
>> - * cleanup and exit
>> - ***************************************************************/
>> - cleanup();
>> -
>> - tst_exit();
>> -}
>> -
>> -/***************************************************************
>> - * help
>> - ***************************************************************/
>> -/* The custom help() function is really simple. Just write your help message to
>> - * standard out. Your help function will be called after the standard options
>> - * have been printed
>> - */
>> -void help(void)
>> -{
>> - printf(" -N #files : create #files files every iteration\n");
>> -}
>> -
>> -/***************************************************************
>> - * setup() - performs all ONE TIME setup for this test.
>> - ***************************************************************/
>> -void setup(void)
>> -{
>> - /* You will want to enable some signal handling so you can capture
>> - * unexpected signals like SIGSEGV.
>> - */
>> - tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -
>> - /* One cavet that hasn't been fixed yet. TEST_PAUSE contains the code to
>> - * fork the test with the -c option. You want to make sure you do this
>> - * before you create your temporary directory.
>> - */
>> - TEST_PAUSE;
>> -
>> - /* If you are doing any file work, you should use a temporary directory. We
>> - * provide tst_tmpdir() which will create a uniquely named temporary
>> - * directory and cd into it. You can now create files in the current
>> - * directory without worrying.
>> - */
>> - tst_tmpdir();
>> -
>> - sprintf(Basename, "%s_%d.", BASENAME, getpid());
>> + if (cnt == nfiles) {
>> + tst_res(TPASS,
>> + "found all %d that were created",
>> + nfiles);
>> + } else {
>> + tst_res(TFAIL,
>> + "found %s files than were created, created: %d, found: %d",
>> + cnt > nfiles ? "more" : "less", nfiles, cnt);
>> + }
>
> Why the newline after TPASS, TFAIL, ? We indent the format string as if
> it continued after the TFAIL/TPASS anyways.
>
--
Regards,
Zhang Lianjie
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-08 14:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 8:32 [LTP] [PATCH 1/2] syscalls/readdir01: Convert to new API zhanglianjie
2021-09-29 8:32 ` [LTP] [PATCH 2/2] syscalls/readdir21: " zhanglianjie
2021-10-07 11:54 ` Cyril Hrubis
2021-10-08 12:42 ` zhanglianjie
2021-10-08 14:43 ` Cyril Hrubis
2021-10-07 11:35 ` [LTP] [PATCH 1/2] syscalls/readdir01: " Cyril Hrubis
2021-10-08 12:37 ` zhanglianjie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.