Menu
▾
▴
Re: [LTP] [PATCH V2 1/2] mount/mount03.c: cleanup
|
From: Eryu G. <eg...@re...> - 2013-05-15 06:13:50
|
On Tue, May 14, 2013 at 10:31:15AM +0800, DAN LI wrote:
>
> 1. Remove useless comments
>
> 2. Revise code to follow ltp-code-style
>
> Signed-off-by: DAN LI <li...@cn...>
Looks good to me.
Reviewed-by: Eryu Guan <eg...@re...>
> ---
> testcases/kernel/syscalls/mount/mount03.c | 213 +++++++++++++-----------------
> 1 file changed, 90 insertions(+), 123 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
> index 04570b9..eea2de7 100644
> --- a/testcases/kernel/syscalls/mount/mount03.c
> +++ b/testcases/kernel/syscalls/mount/mount03.c
> @@ -14,10 +14,8 @@
> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> *
> */
> -/**************************************************************************
> - *
> - * TEST IDENTIFIER : mount03
> - *
> +
> +/*
> * EXECUTED BY : root / superuser
> *
> * TEST TITLE : Test for checking mount(2) flags
> @@ -26,10 +24,6 @@
> *
> * AUTHOR : Nirmala Devi Dhanasekar <nir...@wi...>
> *
> - * SIGNALS
> - * Uses SIGUSR1 to pause before test if option set.
> - * (See the parse_opts(3) man page).
> - *
> * DESCRIPTION
> * Check for basic mount(2) system call flags.
> *
> @@ -42,13 +36,13 @@
> * 5) MS_REMOUNT - alter flags of a mounted FS.
> * 6) MS_NOSUID - ignore suid and sgid bits.
> *
> - * Setup:
> + * Setup:
> * Setup signal handling.
> * Create a mount point.
> * Pause for SIGUSR1 if option specified.
> *
> - * Test:
> - * Loop if the proper options are given.
> + * Test:
> + * Loop if the proper options are given.
> * Execute mount system call for each flag
> * Validate each flag setting. if validation fails
> * Delete the mount point.
> @@ -56,22 +50,10 @@
> * Delete the mount point.
> * Otherwise, Issue a PASS message.
> *
> - * Cleanup:
> - * Print errno log and/or timing stats if options given
> + * Cleanup:
> + * Print errno log and/or timing stats if options given
> * Delete the temporary directory(s)/file(s) created.
> *
> - * USAGE: <for command-line>
> - * mount03 [-T type] -D device [-e] [-i n] [-I x] [-p x] [-t]
> - * where, -T type : specifies the type of filesystem to
> - * be mounted. Default ext2.
> - * -D device : device to be mounted.
> - * -e : Turn on errno logging.
> - * -i n : Execute test n times.
> - * -I x : Execute test for x seconds.
> - * -p : Pause for SIGUSR1 before starting
> - * -P x : Pause for x seconds between iterations.
> - * -t : Turn on syscall timing.
> - *
> * RESTRICTIONS
> * test must run with the -D option
> * test doesn't support -c option to run it in parallel, as mount
> @@ -102,30 +84,29 @@ static int setup_uid(void);
>
> char *TCID = "mount03";
> int TST_TOTAL = 6;
> -extern char **environ; /* pointer to this processes env */
>
> #define DEFAULT_FSTYPE "ext2"
> #define TEMP_FILE "temp_file"
> #define FILE_MODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
> -#define DIR_MODE (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH)
> +#define DIR_MODE (S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
> + S_IXGRP|S_IROTH|S_IXOTH)
> #define SUID_MODE (S_ISUID|S_IRUSR|S_IXUSR|S_IXGRP|S_IXOTH)
>
> -static char *Fstype;
> +static char *fs_type;
>
> static char mntpoint[20];
> static char *fstype;
> static char *device;
> -static int Tflag = 0;
> -static int Dflag = 0;
> -
> -static char write_buffer[BUFSIZ]; /* buffer used to write data to file */
> -static char read_buffer[BUFSIZ]; /* buffer used to read data from file */
> -static int fildes; /* file descriptor for temporary file */
> -static char *Cmd_buffer[3]; /* Buffer to hold command string */
> -static char Path_name[PATH_MAX]; /* Buffer to hold command string */
> -static char testhome_path[PATH_MAX]; /* Test home Path */
> -static char file[PATH_MAX]; /* Temporary file */
> -static char cmd[] = "cp";
> +static int tflag;
> +static int dflag;
> +static int fildes;
> +
> +static char write_buffer[BUFSIZ];
> +static char read_buffer[BUFSIZ];
> +static char path_name[PATH_MAX];
> +static char testhome_path[PATH_MAX];
> +static char file[PATH_MAX];
> +static char *cmd = "cp";
>
> long rwflags[] = {
> MS_RDONLY,
> @@ -136,41 +117,41 @@ long rwflags[] = {
> MS_NOSUID,
> };
>
> -static option_t options[] = { /* options supported by mount03 test */
> - {"T:", &Tflag, &fstype}, /* -T type of filesystem */
> - {"D:", &Dflag, &device}, /* -D device used for mounting */
> +static option_t options[] = {
> + {"T:", &tflag, &fstype},
> + {"D:", &dflag, &device},
> {NULL, NULL, NULL}
> };
>
> -int main(int ac, char **av)
> +int main(int argc, char *argv[])
> {
> int lc, i;
> char *msg;
>
> - if ((msg = parse_opts(ac, av, options, &help)) != NULL)
> + msg = parse_opts(argc, argv, options, &help);
> + if (msg != NULL)
> tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
>
> /* Check for mandatory option of the testcase */
> - if (!Dflag) {
> + if (!dflag)
> tst_brkm(TBROK, NULL,
> "you must specify the device used for mounting with -D "
> "option");
> - }
>
> - if (Tflag) {
> - Fstype = (char *)malloc(strlen(fstype) + 1);
> - if (Fstype == NULL) {
> + if (tflag) {
> + fs_type = malloc(strlen(fstype) + 1);
> + if (fs_type == NULL)
> tst_brkm(TBROK | TERRNO, NULL, "malloc failed");
> - }
> - Fstype[strlen(fstype)] = '\0';
> - strncpy(Fstype, fstype, strlen(fstype));
> +
> + fs_type[strlen(fstype)] = '\0';
> + strncpy(fs_type, fstype, strlen(fstype));
> } else {
> - Fstype = (char *)malloc(strlen(DEFAULT_FSTYPE) + 1);
> - if (Fstype == NULL) {
> + fs_type = malloc(strlen(DEFAULT_FSTYPE) + 1);
> + if (fs_type == NULL)
> tst_brkm(TBROK | TERRNO, NULL, "malloc failed");
> - }
> - strncpy(Fstype, DEFAULT_FSTYPE, strlen(DEFAULT_FSTYPE));
> - Fstype[strlen(DEFAULT_FSTYPE)] = '\0';
> +
> + strncpy(fs_type, DEFAULT_FSTYPE, strlen(DEFAULT_FSTYPE));
> + fs_type[strlen(DEFAULT_FSTYPE)] = '\0';
> }
>
> if (STD_COPIES != 1) {
> @@ -189,7 +170,8 @@ int main(int ac, char **av)
> for (i = 0; i < TST_TOTAL; ++i) {
>
> /* Call mount(2) */
> - TEST(mount(device, mntpoint, Fstype, rwflags[i], NULL));
> + TEST(mount(device, mntpoint, fs_type, rwflags[i],
> + NULL));
>
> /* check return code */
> if (TEST_RETURN != 0) {
> @@ -198,22 +180,20 @@ int main(int ac, char **av)
> }
>
> /* Validate the rwflag */
> - if (test_rwflag(i, lc) == 1) {
> + if (test_rwflag(i, lc) == 1)
> tst_resm(TFAIL, "mount(2) failed while"
> " validating %ld", rwflags[i]);
> - } else {
> + else
> tst_resm(TPASS, "mount(2) passed with "
> "rwflag = %ld", rwflags[i]);
> - }
> +
> TEST(umount(mntpoint));
> - if (TEST_RETURN != 0) {
> + if (TEST_RETURN != 0)
> tst_brkm(TBROK | TTERRNO, cleanup,
> "umount(2) failed for %s", mntpoint);
> - }
> }
> }
>
> - /* cleanup and exit */
> cleanup();
>
> tst_exit();
> @@ -234,8 +214,9 @@ int test_rwflag(int i, int cnt)
> case 0:
> /* Validate MS_RDONLY flag of mount call */
>
> - snprintf(file, PATH_MAX, "%stmp", Path_name);
> - if ((fd = open(file, O_CREAT | O_RDWR, S_IRWXU)) == -1) {
> + snprintf(file, PATH_MAX, "%stmp", path_name);
> + fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> + if (fd == -1) {
> if (errno == EROFS) {
> return 0;
> } else {
> @@ -249,10 +230,11 @@ int test_rwflag(int i, int cnt)
> case 1:
> /* Validate MS_NODEV flag of mount call */
>
> - snprintf(file, PATH_MAX, "%smynod_%d_%d", Path_name, getpid(),
> + snprintf(file, PATH_MAX, "%smynod_%d_%d", path_name, getpid(),
> cnt);
> if (mknod(file, S_IFBLK | 0777, 0) == 0) {
> - if ((fd = open(file, O_RDWR, S_IRWXU)) == -1) {
> + fd = open(file, O_RDWR, S_IRWXU);
> + if (fd == -1) {
> if (errno == EACCES) {
> return 0;
> } else {
> @@ -271,8 +253,9 @@ int test_rwflag(int i, int cnt)
> case 2:
> /* Validate MS_NOEXEC flag of mount call */
>
> - snprintf(file, PATH_MAX, "%stmp1", Path_name);
> - if ((fd = open(file, O_CREAT | O_RDWR, S_IRWXU)) == -1) {
> + snprintf(file, PATH_MAX, "%stmp1", path_name);
> + fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> + if (fd == -1) {
> tst_resm(TWARN | TERRNO, "opening %s failed", file);
> } else {
> close(fd);
> @@ -288,9 +271,9 @@ int test_rwflag(int i, int cnt)
> strcpy(write_buffer, "abcdefghijklmnopqrstuvwxyz");
>
> /* Creat a temporary file under above directory */
> - snprintf(file, PATH_MAX, "%s%s", Path_name, TEMP_FILE);
> - if ((fildes = open(file, O_RDWR | O_CREAT, FILE_MODE))
> - == -1) {
> + snprintf(file, PATH_MAX, "%s%s", path_name, TEMP_FILE);
> + fildes = open(file, O_RDWR | O_CREAT, FILE_MODE);
> + if (fildes == -1) {
> tst_resm(TWARN | TERRNO,
> "open(%s, O_RDWR|O_CREAT, %#o) failed",
> file, FILE_MODE);
> @@ -333,14 +316,14 @@ int test_rwflag(int i, int cnt)
> case 4:
> /* Validate MS_REMOUNT flag of mount call */
>
> - TEST(mount(device, mntpoint, Fstype, MS_REMOUNT, NULL));
> + TEST(mount(device, mntpoint, fs_type, MS_REMOUNT, NULL));
> if (TEST_RETURN != 0) {
> tst_resm(TWARN | TTERRNO, "mount(2) failed to remount");
> return 1;
> } else {
> - snprintf(file, PATH_MAX, "%stmp2", Path_name);
> - if ((fd = open(file, O_CREAT | O_RDWR, S_IRWXU))
> - == -1) {
> + snprintf(file, PATH_MAX, "%stmp2", path_name);
> + fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> + if (fd == -1) {
> tst_resm(TWARN, "open(%s) on readonly "
> "filesystem passed", file);
> return 1;
> @@ -356,23 +339,23 @@ int test_rwflag(int i, int cnt)
> tst_resm(TBROK | TERRNO, "setup_uid failed");
> return 1;
> }
> - switch (pid = fork()) {
> + pid = fork();
> + switch (pid) {
> case -1:
> tst_resm(TBROK | TERRNO, "fork failed");
> return 1;
> case 0:
> - snprintf(file, PATH_MAX, "%ssetuid_test", Path_name);
> - if (chmod(file, SUID_MODE) != 0) {
> + snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
> + if (chmod(file, SUID_MODE) != 0)
> tst_resm(TWARN, "chmod(%s, %#o) failed",
> file, SUID_MODE);
> - }
>
> ltpuser = getpwnam(nobody_uid);
> - if (setreuid(ltpuser->pw_uid, ltpuser->pw_uid) == -1) {
> + if (setreuid(ltpuser->pw_uid, ltpuser->pw_uid) == -1)
> tst_resm(TWARN | TERRNO,
> "seteuid() failed to change euid to %d",
> ltpuser->pw_uid);
> - }
> +
> execlp(file, basename(file), NULL);
> exit(1);
> default:
> @@ -395,24 +378,21 @@ int setup_uid()
> int pid, status;
> char command[PATH_MAX];
>
> - switch (pid = fork()) {
> + pid = fork();
> + switch (pid) {
> case -1:
> tst_resm(TWARN | TERRNO, "fork failed");
> return 1;
> case 0:
> - Cmd_buffer[0] = cmd;
> - Cmd_buffer[1] = testhome_path;
> - Cmd_buffer[2] = Path_name;
> -
> /* Put command into string */
> - sprintf(command, "%s %s %s", cmd, testhome_path, Path_name);
> + sprintf(command, "%s %s %s", cmd, testhome_path, path_name);
>
> /* Run command to cp file to right spot */
> - if (system(command) == 0) {
> + if (system(command) == 0)
> execlp(file, basename(file), NULL);
> - } else {
> + else
> printf("call to %s failed\n", command);
> - }
> +
> exit(1);
> default:
> waitpid(pid, &status, 0);
> @@ -428,61 +408,56 @@ int setup_uid()
> }
> }
>
> -/* setup() - performs all ONE TIME setup for this test */
> void setup()
> {
> - char *test_home; /* variable to hold TESTHOME env */
> + char *test_home;
> struct stat setuid_test_stat;
>
> tst_sig(FORK, DEF_HANDLER, cleanup);
>
> /* Check whether we are root */
> if (geteuid() != 0) {
> - free(Fstype);
> + free(fs_type);
> tst_brkm(TBROK, NULL, "Test must be run as root");
> }
>
> - /* Test home directory */
> test_home = get_current_dir_name();
>
> - /* make a temp directory */
> tst_tmpdir();
>
> - /* Unique mount point */
> - (void)sprintf(mntpoint, "mnt_%d", getpid());
> + sprintf(mntpoint, "mnt_%d", getpid());
>
> - if (mkdir(mntpoint, DIR_MODE)) {
> + if (mkdir(mntpoint, DIR_MODE))
> tst_brkm(TBROK | TERRNO, cleanup, "mkdir(%s, %#o) failed",
> mntpoint, DIR_MODE);
> - }
> +
> /* Get the current working directory of the process */
> - if (getcwd(Path_name, sizeof(Path_name)) == NULL) {
> + if (getcwd(path_name, sizeof(path_name)) == NULL)
> tst_brkm(TBROK, cleanup, "getcwd failed");
> - }
> - if (chmod(Path_name, DIR_MODE) != 0) {
> +
> + if (chmod(path_name, DIR_MODE) != 0)
> tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
> - Path_name, DIR_MODE);
> - }
> - snprintf(file, PATH_MAX, "%ssetuid_test", Path_name);
> + path_name, DIR_MODE);
> +
> + snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
> if (stat(file, &setuid_test_stat) < 0) {
> tst_brkm(TBROK, cleanup, "stat for setuid_test failed");
> } else {
> if ((setuid_test_stat.st_uid || setuid_test_stat.st_gid) &&
> - chown(file, 0, 0) < 0) {
> + chown(file, 0, 0) < 0)
> tst_brkm(TBROK, cleanup,
> "chown for setuid_test failed");
> - }
> +
> if (setuid_test_stat.st_mode != SUID_MODE &&
> - chmod(file, SUID_MODE) < 0) {
> + chmod(file, SUID_MODE) < 0)
> tst_brkm(TBROK, cleanup,
> "setuid for setuid_test failed");
> - }
> }
>
> /*
> * under temporary directory
> */
> - snprintf(Path_name, PATH_MAX, "%s/%s/", Path_name, mntpoint);
> + snprintf(path_name, PATH_MAX, "%s/%s/", path_name, mntpoint);
>
> strcpy(testhome_path, test_home);
> strcat(testhome_path, "/setuid_test");
> @@ -491,18 +466,10 @@ void setup()
>
> }
>
> -/*
> - *cleanup() - performs all ONE TIME cleanup for this test at
> - * completion or premature exit.
> - */
> void cleanup()
> {
> - free(Fstype);
> + free(fs_type);
>
> - /*
> - * print timing stats if that option was specified.
> - * print errno log if that option was specified.
> - */
> TEST_CLEANUP;
>
> tst_rmdir();
> @@ -514,6 +481,6 @@ void cleanup()
> void help()
> {
> printf("-T type : specifies the type of filesystem to be mounted."
> - " Default ext2. \n");
> - printf("-D device : device used for mounting \n");
> + " Default ext2.\n");
> + printf("-D device : device used for mounting.\n");
> }
> --
> 1.8.1
|