[LTP] [PATCH 1/4] syscalls/chmod01: Convert to new API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!

> +/*\
> + * [Description

Missing ] at the end

>   *
>   * Test Description:
>   *  Verify that, chmod(2) succeeds when used to change the mode permissions
> @@ -28,132 +17,50 @@
>   *  chmod(2) should return 0 and the mode permissions set on file should match
>   *  the specified mode.
>   *
> - * Algorithm:
> - *  Setup:
> - *   Setup signal handling.
> - *   Create temporary directory.
> - *   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,
> - *   	Verify the Functionality of system call
> - *      if successful,
> - *      	Issue Functionality-Pass message.
> - *      Otherwise,
> - *		Issue Functionality-Fail message.
> - *  Cleanup:
> - *   Print errno log and/or timing stats if options given
> - *   Delete the temporary directory created.
> - *
> - * Usage:  <for command-line>
> - *  chmod01 [-c n] [-e] [-f] [-i n] [-I x] [-P x] [-t]
> - *     where,  -c n : Run n copies concurrently.
> - *             -e   : Turn on errno logging.
> - *             -f   : Turn off functionality Testing.
> - *	       -i n : Execute test n times.
> - *	       -I x : Execute test for x seconds.
> - *	       -P x : Pause for x seconds between iterations.
> - *	       -t   : Turn on syscall timing.
> - *
> - * HISTORY
> - *	07/2001 Ported by Wayne Boyer
> - *
> - * RESTRICTIONS:
> - *  None.
> - *
>   */
> 
> -#include <stdio.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
> -#include <errno.h>
> -#include <string.h>
> -#include <signal.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> +#include "tst_test.h"
> 
>  #define FILE_MODE	S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
>  #define TESTFILE	"testfile"
> 
> -char *TCID = "chmod01";
> -int TST_TOTAL = 8;
> -
> -int modes[] = { 0, 07, 070, 0700, 0777, 02777, 04777, 06777 };
> +static int modes[] = { 0, 07, 070, 0700, 0777, 02777, 04777, 06777 };
> 
> -void setup();
> -void cleanup();
> -
> -int main(int ac, char **av)
> +static void verify_chmod(unsigned int n)
>  {
>  	struct stat stat_buf;
> -	int lc;
> -	int i;
> -	int mode;
> -
> -	TST_TOTAL = sizeof(modes) / sizeof(int);
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> -
> -		for (i = 0; i < TST_TOTAL; i++) {
> -			mode = modes[i];
> -
> -			TEST(chmod(TESTFILE, mode));
> -
> -			if (TEST_RETURN == -1) {
> -				tst_resm(TFAIL | TTERRNO,
> -					 "chmod(%s, %#o) failed", TESTFILE,
> -					 mode);
> -				continue;
> -			}
> -			if (stat(TESTFILE, &stat_buf) < 0)
> -				tst_brkm(TFAIL | TERRNO, cleanup,
> -					 "stat(%s) failed", TESTFILE);
> -			stat_buf.st_mode &= ~S_IFREG;
> -
> -			if (stat_buf.st_mode == (unsigned int)mode)
> -				tst_resm(TPASS, "Functionality of "
> -					 "chmod(%s, %#o) successful",
> -					 TESTFILE, mode);
> -			else
> -				tst_resm(TFAIL, "%s: Incorrect "
> -					 "modes 0%03o, Expected 0%03o",
> -					 TESTFILE, stat_buf.st_mode,
> -					 mode);
> -		}
> +	int mode = modes[n];
> +
> +	TST_EXP_PASS_SILENT(chmod(TESTFILE, mode), "chmod(%s, %#o) failed",
> +	TESTFILE, mode);

This shouldn't really be the _SILENT() variant since we are testing
chmod here. Also the message should no have the "failed" in it, it
should just describe what is being tested, so this really should just
be:

	TST_EXP_PASS(chmod(TESTFILE, mode),
	             "chmod(%s, %04o)", TESTFILE, mode);


> +	if (TST_PASS) {

This should be:

	if (!TST_PASS)
		return;

So that we don't have to have the rest of the test inside an if ()
block.

> +		SAFE_STAT(TESTFILE, &stat_buf);
> +		stat_buf.st_mode &= ~S_IFREG;
> +
> +		if (stat_buf.st_mode == (unsigned int)mode)
> +			tst_res(TPASS, "Functionality of "
> +					"chmod(%s, %#o) successful",
> +					TESTFILE, mode);
> +		else
> +			tst_res(TFAIL, "%s: Incorrect "
> +					"modes 0%03o, Expected 0%03o",
> +					TESTFILE, stat_buf.st_mode,
> +					mode);
>  	}

And these messages could be shorter and to the point. If we do not
silence the output of the TST_EXP_PASS() above we can just change these
to:

        if (stat_buf.st_mode == (unsigned int)mode) {
                tst_res(TPASS, "stat(%s) mode=%04o",
                        TESTFILE, stat_buf.st_mode);
        } else {
                tst_res(TFAIL, "stat(%s) mode=%04o",
                       TESTFILE, stat_buf.st_mode);
        }

And the rest output would look like:

chmod01.c:32: TPASS: chmod(testfile, 0000) passed
chmod01.c:42: TPASS: stat(testfile) mode=0000
chmod01.c:32: TPASS: chmod(testfile, 0007) passed
chmod01.c:42: TPASS: stat(testfile) mode=0007
...


> -	cleanup();
> -	tst_exit();
>  }
> 
> -void setup(void)
> +static void setup(void)
>  {
>  	int fd;
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	tst_tmpdir();
> -
> -	fd = SAFE_OPEN(cleanup, TESTFILE, O_RDWR | O_CREAT, FILE_MODE);
> -	SAFE_CLOSE(cleanup, fd);
> -
> +	fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, FILE_MODE);
> +	SAFE_CLOSE(fd);

We can just do:

        SAFE_TOUCH(TESTFILE, FILE_MODE, NULL);


>  }
> 
> -void cleanup(void)
> -{
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +	.setup = setup,
> +	.tcnt = ARRAY_SIZE(modes),
> +	.test = verify_chmod,
> +	.needs_root = 1,

As far as I can tell there is no reason why this test couldn't be
executed as a normal user, so we shouldn't set the needs_root here.

> +	.needs_tmpdir = 1,
> +};
> +
> --
> 2.20.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz