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

[Cyril Hrubis]

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