* [LTP] [PATCH 1/2]mount/mount03.c: cleanup
@ 2013-05-13 2:41 DAN LI
2013-05-13 3:12 ` [LTP] [PATCH 2/2]mount/mount03.c: fix several issues DAN LI
2013-05-13 8:26 ` [LTP] [PATCH 1/2]mount/mount03.c: cleanup Eryu Guan
0 siblings, 2 replies; 6+ messages in thread
From: DAN LI @ 2013-05-13 2:41 UTC (permalink / raw)
To: LTP list
1. Remove useless comments
2. Revise code to follow ltp-code-style
Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
---
testcases/kernel/syscalls/mount/mount03.c | 208 +++++++++++++-----------------
1 file changed, 89 insertions(+), 119 deletions(-)
diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
index 04570b9..ea4b0e9 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 <nirmala.devi@wipro.com>
*
- * 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 char write_buffer[BUFSIZ];
+static char read_buffer[BUFSIZ];
+static int fildes;
+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 = (char *)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 = (char *)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,17 +408,16 @@ 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");
}
@@ -451,38 +430,37 @@ void setup()
/* Unique mount point */
(void)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 +469,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 +484,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.3
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH 2/2]mount/mount03.c: fix several issues
2013-05-13 2:41 [LTP] [PATCH 1/2]mount/mount03.c: cleanup DAN LI
@ 2013-05-13 3:12 ` DAN LI
2013-05-13 8:41 ` Eryu Guan
2013-05-13 8:26 ` [LTP] [PATCH 1/2]mount/mount03.c: cleanup Eryu Guan
1 sibling, 1 reply; 6+ messages in thread
From: DAN LI @ 2013-05-13 3:12 UTC (permalink / raw)
To: LTP list
Make following fixes:
1. In setup():
Create the file before calling stat(file).
Call get_current_dir_name() after tst_tmpdir().
Fix incorrect using of snprintf().
2. For MS_NOEXEC test, change judgement method to
If errno is set to EACCES after execlp(exec-file), this case passes.
Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
---
testcases/kernel/syscalls/mount/mount03.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
index ea4b0e9..1dc39f2 100644
--- a/testcases/kernel/syscalls/mount/mount03.c
+++ b/testcases/kernel/syscalls/mount/mount03.c
@@ -206,7 +206,7 @@ int main(int argc, char *argv[])
int test_rwflag(int i, int cnt)
{
- int fd, pid, status;
+ int ret, fd, pid, status;
char nobody_uid[] = "nobody";
struct passwd *ltpuser;
@@ -259,7 +259,9 @@ int test_rwflag(int i, int cnt)
tst_resm(TWARN | TERRNO, "opening %s failed", file);
} else {
close(fd);
- execlp(file, basename(file), NULL);
+ ret = execlp(file, basename(file), NULL);
+ if ((ret == -1) && (errno == EACCES))
+ return 0;
}
return 1;
case 3:
@@ -410,6 +412,8 @@ int setup_uid()
void setup()
{
+ int fd;
+ char path[PATH_MAX];
char *test_home;
struct stat setuid_test_stat;
@@ -421,12 +425,12 @@ void setup()
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();
+ /* Test home directory */
+ test_home = get_current_dir_name();
+
/* Unique mount point */
(void)sprintf(mntpoint, "mnt_%d", getpid());
@@ -442,7 +446,12 @@ void setup()
tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
path_name, DIR_MODE);
- snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
+ snprintf(file, PATH_MAX, "%s/setuid_test", path_name);
+ fd = open(file, O_CREAT | O_TRUNC);
+ if (fd == -1)
+ tst_brkm(TBROK, cleanup, "open file failed");
+ close(fd);
+
if (stat(file, &setuid_test_stat) < 0) {
tst_brkm(TBROK, cleanup, "stat for setuid_test failed");
} else {
@@ -460,8 +469,8 @@ void setup()
/*
* under temporary directory
*/
- snprintf(path_name, PATH_MAX, "%s/%s/", path_name, mntpoint);
-
+ strncpy(path, path_name, PATH_MAX);
+ snprintf(path_name, PATH_MAX, "%s/%s/", path, mntpoint);
strcpy(testhome_path, test_home);
strcat(testhome_path, "/setuid_test");
--
1.8.3
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH 1/2]mount/mount03.c: cleanup
2013-05-13 2:41 [LTP] [PATCH 1/2]mount/mount03.c: cleanup DAN LI
2013-05-13 3:12 ` [LTP] [PATCH 2/2]mount/mount03.c: fix several issues DAN LI
@ 2013-05-13 8:26 ` Eryu Guan
2013-05-13 10:37 ` DAN LI
1 sibling, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2013-05-13 8:26 UTC (permalink / raw)
To: DAN LI; +Cc: LTP list
On Mon, May 13, 2013 at 10:41:39AM +0800, DAN LI wrote:
>
> 1. Remove useless comments
>
> 2. Revise code to follow ltp-code-style
>
> Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
> ---
> testcases/kernel/syscalls/mount/mount03.c | 208 +++++++++++++-----------------
> 1 file changed, 89 insertions(+), 119 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
> index 04570b9..ea4b0e9 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 <nirmala.devi@wipro.com>
> *
> - * 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|\
^^^^ I'd like a space here, | \
> + 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 char write_buffer[BUFSIZ];
> +static char read_buffer[BUFSIZ];
> +static int fildes;
move this up and put all int vars together?
> +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 = (char *)malloc(strlen(fstype) + 1);
^^^^^^^^ not sure if we really need a cast for malloc
> + 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 = (char *)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));
indent like this?
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,17 +408,16 @@ 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");
> }
>
> @@ -451,38 +430,37 @@ void setup()
> /* Unique mount point */
> (void)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 +469,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 +484,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");
^^ we need a space here otherwise the output will be:
"... to be mounted.Default ext2."
^^^^^ no space
Thanks for the cleanup!
Eryu Guan
> + printf("-D device : device used for mounting\n");
> }
> --
> 1.8.3
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH 2/2]mount/mount03.c: fix several issues
2013-05-13 3:12 ` [LTP] [PATCH 2/2]mount/mount03.c: fix several issues DAN LI
@ 2013-05-13 8:41 ` Eryu Guan
2013-05-13 10:58 ` DAN LI
0 siblings, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2013-05-13 8:41 UTC (permalink / raw)
To: DAN LI; +Cc: LTP list
Hi,
For patch summary
[LTP] [PATCH 2/2]mount/mount03.c: fix several issues
^^ ^^ one extra space
^^ one space is needed here
On Mon, May 13, 2013 at 11:12:11AM +0800, DAN LI wrote:
> Make following fixes:
> 1. In setup():
> Create the file before calling stat(file).
> Call get_current_dir_name() after tst_tmpdir().
> Fix incorrect using of snprintf().
>
> 2. For MS_NOEXEC test, change judgement method to
> If errno is set to EACCES after execlp(exec-file), this case passes.
I don't think you need to indent commit message like this.
>
> Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
> ---
> testcases/kernel/syscalls/mount/mount03.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
> index ea4b0e9..1dc39f2 100644
> --- a/testcases/kernel/syscalls/mount/mount03.c
> +++ b/testcases/kernel/syscalls/mount/mount03.c
> @@ -206,7 +206,7 @@ int main(int argc, char *argv[])
>
> int test_rwflag(int i, int cnt)
> {
> - int fd, pid, status;
> + int ret, fd, pid, status;
> char nobody_uid[] = "nobody";
> struct passwd *ltpuser;
>
> @@ -259,7 +259,9 @@ int test_rwflag(int i, int cnt)
> tst_resm(TWARN | TERRNO, "opening %s failed", file);
> } else {
> close(fd);
> - execlp(file, basename(file), NULL);
> + ret = execlp(file, basename(file), NULL);
> + if ((ret == -1) && (errno == EACCES))
> + return 0;
Should we give out a TWARN or TBROK when execlp failed?
> }
> return 1;
> case 3:
> @@ -410,6 +412,8 @@ int setup_uid()
>
> void setup()
> {
> + int fd;
> + char path[PATH_MAX];
> char *test_home;
> struct stat setuid_test_stat;
>
> @@ -421,12 +425,12 @@ void setup()
> 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();
>
> + /* Test home directory */
Unwanted comment I think
> + test_home = get_current_dir_name();
> +
> /* Unique mount point */
here too
> (void)sprintf(mntpoint, "mnt_%d", getpid());
^^^^^^ this is useless
All the three changes should go in the first cleanup patch.
Thanks,
Eryu Guan
>
> @@ -442,7 +446,12 @@ void setup()
> tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
> path_name, DIR_MODE);
>
> - snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
> + snprintf(file, PATH_MAX, "%s/setuid_test", path_name);
> + fd = open(file, O_CREAT | O_TRUNC);
> + if (fd == -1)
> + tst_brkm(TBROK, cleanup, "open file failed");
> + close(fd);
> +
> if (stat(file, &setuid_test_stat) < 0) {
> tst_brkm(TBROK, cleanup, "stat for setuid_test failed");
> } else {
> @@ -460,8 +469,8 @@ void setup()
> /*
> * under temporary directory
> */
> - snprintf(path_name, PATH_MAX, "%s/%s/", path_name, mntpoint);
> -
> + strncpy(path, path_name, PATH_MAX);
> + snprintf(path_name, PATH_MAX, "%s/%s/", path, mntpoint);
> strcpy(testhome_path, test_home);
> strcat(testhome_path, "/setuid_test");
>
> --
> 1.8.3
>
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH 1/2]mount/mount03.c: cleanup
2013-05-13 8:26 ` [LTP] [PATCH 1/2]mount/mount03.c: cleanup Eryu Guan
@ 2013-05-13 10:37 ` DAN LI
0 siblings, 0 replies; 6+ messages in thread
From: DAN LI @ 2013-05-13 10:37 UTC (permalink / raw)
To: Eryu Guan; +Cc: LTP list
On 05/13/2013 04:26 PM, Eryu Guan wrote:
> On Mon, May 13, 2013 at 10:41:39AM +0800, DAN LI wrote:
>>
>> 1. Remove useless comments
>>
>> 2. Revise code to follow ltp-code-style
>>
>> Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
>> ---
>> testcases/kernel/syscalls/mount/mount03.c | 208 +++++++++++++-----------------
>> 1 file changed, 89 insertions(+), 119 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
>> index 04570b9..ea4b0e9 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 <nirmala.devi@wipro.com>
>> *
>> - * 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|\
> ^^^^ I'd like a space here, | \
Agreed.
>> + 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 char write_buffer[BUFSIZ];
>> +static char read_buffer[BUFSIZ];
>> +static int fildes;
>
> move this up and put all int vars together?
Agreed.
>> +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 = (char *)malloc(strlen(fstype) + 1);
> ^^^^^^^^ not sure if we really need a cast for malloc
>
It's a watch point in C++, but for C, maybe better to cut the cast.
>> + 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 = (char *)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));
> indent like this?
> TEST(mount(device, mntpoint, fs_type, rwflags[i],
> NULL));
Agreed.
>>
>> /* 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,17 +408,16 @@ 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");
>> }
>>
>> @@ -451,38 +430,37 @@ void setup()
>> /* Unique mount point */
>> (void)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 +469,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 +484,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");
> ^^ we need a space here otherwise the output will be:
> "... to be mounted.Default ext2."
> ^^^^^ no space
>
But it seems that checkpatch.pl do not agree it.
I'will check it.
Thanks for reviewing.
DAN LI
> Thanks for the cleanup!
>
> Eryu Guan
>> + printf("-D device : device used for mounting\n");
>> }
>> --
>> 1.8.3
>>
>> ------------------------------------------------------------------------------
>> Learn Graph Databases - Download FREE O'Reilly Book
>> "Graph Databases" is the definitive new guide to graph databases and
>> their applications. This 200-page book is written by three acclaimed
>> leaders in the field. The early access version is available now.
>> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH 2/2]mount/mount03.c: fix several issues
2013-05-13 8:41 ` Eryu Guan
@ 2013-05-13 10:58 ` DAN LI
0 siblings, 0 replies; 6+ messages in thread
From: DAN LI @ 2013-05-13 10:58 UTC (permalink / raw)
To: Eryu Guan; +Cc: LTP list
On 05/13/2013 04:41 PM, Eryu Guan wrote:
> Hi,
>
> For patch summary
> [LTP] [PATCH 2/2]mount/mount03.c: fix several issues
> ^^ ^^ one extra space
> ^^ one space is needed here
Ok.
> On Mon, May 13, 2013 at 11:12:11AM +0800, DAN LI wrote:
>> Make following fixes:
>> 1. In setup():
>> Create the file before calling stat(file).
>> Call get_current_dir_name() after tst_tmpdir().
>> Fix incorrect using of snprintf().
>>
>> 2. For MS_NOEXEC test, change judgement method to
>> If errno is set to EACCES after execlp(exec-file), this case passes.
>
> I don't think you need to indent commit message like this.
Agreed.
>>
>> Signed-off-by: DAN LI <li.dan@cn.fujitsu.com>
>> ---
>> testcases/kernel/syscalls/mount/mount03.c | 25 +++++++++++++++++--------
>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
>> index ea4b0e9..1dc39f2 100644
>> --- a/testcases/kernel/syscalls/mount/mount03.c
>> +++ b/testcases/kernel/syscalls/mount/mount03.c
>> @@ -206,7 +206,7 @@ int main(int argc, char *argv[])
>>
>> int test_rwflag(int i, int cnt)
>> {
>> - int fd, pid, status;
>> + int ret, fd, pid, status;
>> char nobody_uid[] = "nobody";
>> struct passwd *ltpuser;
>>
>> @@ -259,7 +259,9 @@ int test_rwflag(int i, int cnt)
>> tst_resm(TWARN | TERRNO, "opening %s failed", file);
>> } else {
>> close(fd);
>> - execlp(file, basename(file), NULL);
>> + ret = execlp(file, basename(file), NULL);
>> + if ((ret == -1) && (errno == EACCES))
>> + return 0;
> Should we give out a TWARN or TBROK when execlp failed?
This failure is the expected result. So, an error message is may
not necessary.
>
>> }
>> return 1;
>> case 3:
>> @@ -410,6 +412,8 @@ int setup_uid()
>>
>> void setup()
>> {
>> + int fd;
>> + char path[PATH_MAX];
>> char *test_home;
>> struct stat setuid_test_stat;
>>
>> @@ -421,12 +425,12 @@ void setup()
>> 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();
>>
>> + /* Test home directory */
> Unwanted comment I think
Agreed.
>
>> + test_home = get_current_dir_name();
>> +
>> /* Unique mount point */
> here too
Agreed.
>
>> (void)sprintf(mntpoint, "mnt_%d", getpid());
> ^^^^^^ this is useless
Agreed.
>
> All the three changes should go in the first cleanup patch.
Ok.
I'll fix all of them in coming V2.
Thanks again for reviewing.
DAN LI
>
> Thanks,
> Eryu Guan
>>
>> @@ -442,7 +446,12 @@ void setup()
>> tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
>> path_name, DIR_MODE);
>>
>> - snprintf(file, PATH_MAX, "%ssetuid_test", path_name);
>> + snprintf(file, PATH_MAX, "%s/setuid_test", path_name);
>> + fd = open(file, O_CREAT | O_TRUNC);
>> + if (fd == -1)
>> + tst_brkm(TBROK, cleanup, "open file failed");
>> + close(fd);
>> +
>> if (stat(file, &setuid_test_stat) < 0) {
>> tst_brkm(TBROK, cleanup, "stat for setuid_test failed");
>> } else {
>> @@ -460,8 +469,8 @@ void setup()
>> /*
>> * under temporary directory
>> */
>> - snprintf(path_name, PATH_MAX, "%s/%s/", path_name, mntpoint);
>> -
>> + strncpy(path, path_name, PATH_MAX);
>> + snprintf(path_name, PATH_MAX, "%s/%s/", path, mntpoint);
>> strcpy(testhome_path, test_home);
>> strcat(testhome_path, "/setuid_test");
>>
>> --
>> 1.8.3
>>
>>
>> ------------------------------------------------------------------------------
>> Learn Graph Databases - Download FREE O'Reilly Book
>> "Graph Databases" is the definitive new guide to graph databases and
>> their applications. This 200-page book is written by three acclaimed
>> leaders in the field. The early access version is available now.
>> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
>> _______________________________________________
>> Ltp-list mailing list
>> Ltp-list@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-13 11:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-13 2:41 [LTP] [PATCH 1/2]mount/mount03.c: cleanup DAN LI
2013-05-13 3:12 ` [LTP] [PATCH 2/2]mount/mount03.c: fix several issues DAN LI
2013-05-13 8:41 ` Eryu Guan
2013-05-13 10:58 ` DAN LI
2013-05-13 8:26 ` [LTP] [PATCH 1/2]mount/mount03.c: cleanup Eryu Guan
2013-05-13 10:37 ` DAN LI
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.