Skip to contentSkip to conversation

mount/mount03.c: clean up

PATCH Superseded DAN LI 2 participants 5 messages Jul 2, 2013 – Jul 9, 2013

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

DAN LI

author
#0 lore raw

PATCH 1/2 mount/mount03.c: clean up

Clean up mount03.c:
xxx func() -> xxx func(void)

clean up setup()

---
 testcases/kernel/syscalls/mount/mount03.c | 47 ++++++++++---------------------
 1 file changed, 15 insertions(+), 32 deletions(-)
testcases/kernel/syscalls/mount/mount03.c +15 -32
index b1aa71f..8b2d3e4 100644
@@ -85,6 +85,7 @@ static int setup_uid(void);
 char *TCID = "mount03";
 int TST_TOTAL = 6;
 
+#define EXEC_COMMAND	"cp"
 #define DEFAULT_FSTYPE	"ext2"
 #define TEMP_FILE	"temp_file"
 #define FILE_MODE	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
@@ -106,7 +107,6 @@ 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,
@@ -375,7 +375,7 @@ int test_rwflag(int i, int cnt)
 }
 
 /* setup_uid() - performs setup for NOUID test */
-int setup_uid()
+int setup_uid(void)
 {
 	int pid, status;
 	char command[PATH_MAX];
@@ -387,7 +387,8 @@ int setup_uid()
 		return 1;
 	case 0:
 		/* Put command into string */
-		sprintf(command, "%s %s %s", cmd, testhome_path, path_name);
+		sprintf(command, "%s %s %s",
+			EXEC_COMMAND, testhome_path, path_name);
 
 		/* Run command to cp file to right spot */
 		if (system(command) == 0)
@@ -410,25 +411,17 @@ int setup_uid()
 	}
 }
 
-void setup()
+void setup(void)
 {
-	int fd;
 	char path[PATH_MAX];
-	char *test_home;
 	struct stat setuid_test_stat;
 
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
-	/* Check whether we are root */
-	if (geteuid() != 0) {
-		free(fs_type);
-		tst_brkm(TBROK, NULL, "Test must be run as root");
-	}
+	tst_require_root(NULL);
 
 	tst_tmpdir();
 
-	test_home = get_current_dir_name();
-
 	sprintf(mntpoint, "mnt_%d", getpid());
 
 	if (mkdir(mntpoint, DIR_MODE))
@@ -444,38 +437,28 @@ void setup()
 			 path_name, DIR_MODE);
 
 	snprintf(file, PATH_MAX, "%s/setuid_test", path_name);
-	fd = open(file, O_CREAT | O_TRUNC, S_IRWXU);
-	if (fd == -1)
+	if (open(file, O_CREAT | O_TRUNC, S_IRWXU) == -1)
 		tst_brkm(TBROK, cleanup, "open file failed");
-	close(fd);
 
-	if (stat(file, &setuid_test_stat) < 0) {
+	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)
-			tst_brkm(TBROK, cleanup,
-				 "chown for setuid_test failed");
-
-		if (setuid_test_stat.st_mode != SUID_MODE &&
-		    chmod(file, SUID_MODE) < 0)
-			tst_brkm(TBROK, cleanup,
-				 "setuid for setuid_test failed");
-	}
+
+	if (setuid_test_stat.st_mode != SUID_MODE && chmod(file, SUID_MODE) < 0)
+		tst_brkm(TBROK, cleanup,
+			 "setuid for setuid_test failed");
 
 	/*
 	 * under temporary directory
 	 */
+	strcpy(testhome_path, file);
 	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");
 
 	TEST_PAUSE;
 
 }
 
-void cleanup()
+void cleanup(void)
 {
 	free(fs_type);
 
@@ -487,7 +470,7 @@ void cleanup()
 /*
  * issue a help message
  */
-void help()
+void help(void)
 {
 	printf("-T type	  : specifies the type of filesystem to be mounted."
 	       " Default ext2.\n");
Signature · 12 lines
1.8.1

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
author
#1 lore raw

PATCH 2/2 mount/mount03.c: Test feature MS_NOATIME of mount(2)

Additional test for features MS_NOATIME.

---
 testcases/kernel/syscalls/mount/mount03.c | 46 +++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/mount/mount03.c +44 -2
index 8b2d3e4..cb99887 100644
@@ -20,7 +20,7 @@
  *
  *    TEST TITLE	: Test for checking mount(2) flags
  *
- *    TEST CASE TOTAL	: 6
+ *    TEST CASE TOTAL	: 7
  *
  *    AUTHOR		: Nirmala Devi Dhanasekar <[email protected]>
  *
@@ -35,6 +35,7 @@
  *	4) MS_SYNCHRONOUS - writes are synced at once.
  *	5) MS_REMOUNT - alter flags of a mounted FS.
  *	6) MS_NOSUID - ignore suid and sgid bits.
+ *	7) MS_NOATIME - do not update access times.
  *
  *	Setup:
  *	  Setup signal handling.
@@ -83,7 +84,7 @@ static int test_rwflag(int, int);
 static int setup_uid(void);
 
 char *TCID = "mount03";
-int TST_TOTAL = 6;
+int TST_TOTAL = 7;
 
 #define EXEC_COMMAND	"cp"
 #define DEFAULT_FSTYPE	"ext2"
@@ -115,6 +116,7 @@ long rwflags[] = {
 	MS_SYNCHRONOUS,
 	MS_RDONLY,
 	MS_NOSUID,
+	MS_NOATIME,
 };
 
 static option_t options[] = {
@@ -208,6 +210,8 @@ int test_rwflag(int i, int cnt)
 {
 	int ret, fd, pid, status;
 	char nobody_uid[] = "nobody";
+	time_t atime;
+	struct stat file_stat;
 	struct passwd *ltpuser;
 
 	switch (i) {
@@ -370,6 +374,44 @@ int test_rwflag(int i, int cnt)
 					return 1;
 			}
 		}
+	case 6:
+		/* Validate MS_NOATIME flag of mount call */
+
+		snprintf(file, PATH_MAX, "%satime", path_name);
+		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
+		if (fd == -1) {
+			tst_resm(TWARN | TERRNO, "opening %s failed", file);
+			return 1;
+		}
+
+		if (write(fd, "TEST_MS_NOATIME", 15) != 15) {
+			tst_resm(TWARN | TERRNO, "write %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #1", file);
+			return 1;
+		}
+
+		atime = file_stat.st_atime;
+
+		if (read(fd, NULL, 20) == -1) {
+			tst_resm(TWARN | TERRNO, "read %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #2", file);
+			return 1;
+		}
+		close(fd);
+
+		if (file_stat.st_atime != atime) {
+			tst_resm(TWARN, "access time is updated");
+			return 1;
+		}
+		return 0;
 	}
 	return 0;
 }
Signature · 12 lines
1.8.1

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
author
#2 lore raw

PATCH v2 2/2 mount/mount03.c: Test feature MS_NOATIME of mount(2)

In reply to DAN LI

Additional test for features MS_NOATIME.

---
 testcases/kernel/syscalls/mount/mount03.c | 48 +++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)
testcases/kernel/syscalls/mount/mount03.c +46 -2
index 8b2d3e4..6974107 100644
@@ -20,7 +20,7 @@
  *
  *    TEST TITLE	: Test for checking mount(2) flags
  *
- *    TEST CASE TOTAL	: 6
+ *    TEST CASE TOTAL	: 7
  *
  *    AUTHOR		: Nirmala Devi Dhanasekar <[email protected]>
  *
@@ -35,6 +35,7 @@
  *	4) MS_SYNCHRONOUS - writes are synced at once.
  *	5) MS_REMOUNT - alter flags of a mounted FS.
  *	6) MS_NOSUID - ignore suid and sgid bits.
+ *	7) MS_NOATIME - do not update access times.
  *
  *	Setup:
  *	  Setup signal handling.
@@ -83,7 +84,7 @@ static int test_rwflag(int, int);
 static int setup_uid(void);
 
 char *TCID = "mount03";
-int TST_TOTAL = 6;
+int TST_TOTAL = 7;
 
 #define EXEC_COMMAND	"cp"
 #define DEFAULT_FSTYPE	"ext2"
@@ -115,6 +116,7 @@ long rwflags[] = {
 	MS_SYNCHRONOUS,
 	MS_RDONLY,
 	MS_NOSUID,
+	MS_NOATIME,
 };
 
 static option_t options[] = {
@@ -208,6 +210,8 @@ int test_rwflag(int i, int cnt)
 {
 	int ret, fd, pid, status;
 	char nobody_uid[] = "nobody";
+	time_t atime;
+	struct stat file_stat;
 	struct passwd *ltpuser;
 
 	switch (i) {
@@ -370,6 +374,46 @@ int test_rwflag(int i, int cnt)
 					return 1;
 			}
 		}
+	case 6:
+		/* Validate MS_NOATIME flag of mount call */
+
+		snprintf(file, PATH_MAX, "%satime", path_name);
+		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
+		if (fd == -1) {
+			tst_resm(TWARN | TERRNO, "opening %s failed", file);
+			return 1;
+		}
+
+		if (write(fd, "TEST_MS_NOATIME", 15) != 15) {
+			tst_resm(TWARN | TERRNO, "write %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #1", file);
+			return 1;
+		}
+
+		atime = file_stat.st_atime;
+
+		sleep(1);
+
+		if (read(fd, NULL, 20) == -1) {
+			tst_resm(TWARN | TERRNO, "read %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #2", file);
+			return 1;
+		}
+		close(fd);
+
+		if (file_stat.st_atime != atime) {
+			tst_resm(TWARN, "access time is updated");
+			return 1;
+		}
+		return 0;
 	}
 	return 0;
 }
Signature · 14 lines
1.8.1



------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#3 lore raw

Re: [LTP] [PATCH 2/2 v2] mount/mount03.c: Test feature MS_NOATIME of mount(2)

Hi!

Quoted 35 lines

Additional test for features MS_NOATIME.

---

testcases/kernel/syscalls/mount/mount03.c +31 -0
+	case 6:
+		/* Validate MS_NOATIME flag of mount call */
+
+		snprintf(file, PATH_MAX, "%satime", path_name);
+		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
+		if (fd == -1) {
+			tst_resm(TWARN | TERRNO, "opening %s failed", file);
+			return 1;
+		}
+
+		if (write(fd, "TEST_MS_NOATIME", 15) != 15) {
+			tst_resm(TWARN | TERRNO, "write %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #1", file);
+			return 1;
+		}
+
+		atime = file_stat.st_atime;
+
+		sleep(1);
+
+		if (read(fd, NULL, 20) == -1) {
+			tst_resm(TWARN | TERRNO, "read %s failed", file);
+			return 1;
+		}
+
+		if (fstat(fd, &file_stat) == -1) {
+			tst_resm(TWARN | TERRNO, "stat %s failed #2", file);

testcases/kernel/syscalls/mount/mount03.c int test_rwflag(int i, int cnt) · line 417

Quoted 10 lines
testcases/kernel/syscalls/mount/mount03.c +9 -0
+			return 1;
+		}
+		close(fd);
+
+		if (file_stat.st_atime != atime) {
+			tst_resm(TWARN, "access time is updated");
+			return 1;
+		}
+		return 0;
 	}

This version looks fine.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#4 lore raw
In reply to DAN LI

Hi!

Quoted 7 lines

Clean up mount03.c:
xxx func() -> xxx func(void)

clean up setup()

Sorry it took longer than usuall.

Quoted 33 lines
 testcases/kernel/syscalls/mount/mount03.c | 47 ++++++++++---------------------
 1 file changed, 15 insertions(+), 32 deletions(-)
testcases/kernel/syscalls/mount/mount03.c +2 -2
index b1aa71f..8b2d3e4 100644
@@ -85,6 +85,7 @@ static int setup_uid(void);
 char *TCID = "mount03";
 int TST_TOTAL = 6;
 
+#define EXEC_COMMAND	"cp"
 #define DEFAULT_FSTYPE	"ext2"
 #define TEMP_FILE	"temp_file"
 #define FILE_MODE	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
@@ -106,7 +107,6 @@ 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,
@@ -375,7 +375,7 @@ int test_rwflag(int i, int cnt)
 }
 
 /* setup_uid() - performs setup for NOUID test */
-int setup_uid()
+int setup_uid(void)
 {
 	int pid, status;
 	char command[PATH_MAX];

testcases/kernel/syscalls/mount/mount03.c int setup_uid() · line 394

Quoted 9 lines
testcases/kernel/syscalls/mount/mount03.c +2 -1
@@ -387,7 +387,8 @@ int setup_uid()
 		return 1;
 	case 0:
 		/* Put command into string */
-		sprintf(command, "%s %s %s", cmd, testhome_path, path_name);
+		sprintf(command, "%s %s %s",
+			EXEC_COMMAND, testhome_path, path_name);
 
 		/* Run command to cp file to right spot */
 		if (system(command) == 0)

What about using SAFE_CP() from safe_file_ops.h?

Quoted 16 lines
testcases/kernel/syscalls/mount/mount03.c +1 -5
@@ -410,25 +411,17 @@ int setup_uid()
 	}
 }
 
-void setup()
+void setup(void)
 {
-	int fd;
 	char path[PATH_MAX];
-	char *test_home;
 	struct stat setuid_test_stat;
 
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
-	/* Check whether we are root */
-	if (geteuid() != 0) {

testcases/kernel/syscalls/mount/mount03.c int setup_uid() · line 425

Quoted 10 lines
testcases/kernel/syscalls/mount/mount03.c +1 -5
-		free(fs_type);
-		tst_brkm(TBROK, NULL, "Test must be run as root");
-	}
+	tst_require_root(NULL);
 
 	tst_tmpdir();
 
-	test_home = get_current_dir_name();
-
 	sprintf(mntpoint, "mnt_%d", getpid());

This getpid() shouldn't be needed as we are in the test tmpdir anyway.

testcases/kernel/syscalls/mount/mount03.c void setup() · line 450

Quoted 10 lines
testcases/kernel/syscalls/mount/mount03.c +1 -3
 	if (mkdir(mntpoint, DIR_MODE))
@@ -444,38 +437,28 @@ void setup()
 			 path_name, DIR_MODE);
 
 	snprintf(file, PATH_MAX, "%s/setuid_test", path_name);
-	fd = open(file, O_CREAT | O_TRUNC, S_IRWXU);
-	if (fd == -1)
+	if (open(file, O_CREAT | O_TRUNC, S_IRWXU) == -1)
 		tst_brkm(TBROK, cleanup, "open file failed");
-	close(fd);

Why removing the close(fd)?

Quoted 8 lines
testcases/kernel/syscalls/mount/mount03.c +1 -6
-	if (stat(file, &setuid_test_stat) < 0) {
+	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)
-			tst_brkm(TBROK, cleanup,
-				 "chown for setuid_test failed");

testcases/kernel/syscalls/mount/mount03.c void setup() · line 448

Quoted 10 lines
testcases/kernel/syscalls/mount/mount03.c +4 -6
-
-		if (setuid_test_stat.st_mode != SUID_MODE &&
-		    chmod(file, SUID_MODE) < 0)
-			tst_brkm(TBROK, cleanup,
-				 "setuid for setuid_test failed");
-	}
+
+	if (setuid_test_stat.st_mode != SUID_MODE && chmod(file, SUID_MODE) < 0)
+		tst_brkm(TBROK, cleanup,
+			 "setuid for setuid_test failed");

^
This could now be one line statement if I'm not mistaken

Quoted 20 lines
testcases/kernel/syscalls/mount/mount03.c +2 -3
 	/*
 	 * under temporary directory
 	 */
+	strcpy(testhome_path, file);
 	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");
 
 	TEST_PAUSE;
 
 }
 
-void cleanup()
+void cleanup(void)
 {
 	free(fs_type);
 

testcases/kernel/syscalls/mount/mount03.c void cleanup() · line 476

Quoted 8 lines
testcases/kernel/syscalls/mount/mount03.c +1 -1
@@ -487,7 +470,7 @@ void cleanup()
 /*
  * issue a help message
  */
-void help()
+void help(void)
 {
 	printf("-T type	  : specifies the type of filesystem to be mounted."
 	       " Default ext2.\n");

Also please make all internal functions static.

And pretty please remove the absurd fs_type flag allocation.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list