Skip to contentSkip to conversation

Misc cleanups (new lib) and fixes for Android

PATCH Superseded Sandeep Patil 3 participants 15 messages Nov 5, 2018 – Nov 6, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Misc cleanups (new lib) and fixes for Android

The following series contains patches to cleanup some of the old tests and make use of the new library in them. That came with additional benefit of getting easy ways to make those tests run on Android systems.

Note that some of these tests were made to run by hard coding for ANDROID in them and that now goes away too.[1]

Sandeep Patil (5):
chroot01: Use the 'tmpdir' we create. syscalls/chmod07: Use new ltp library syscalls/fchmod02: fall back to use "daemon" group if "users" is
absent.
syscalls/chmod05: Use new ltp library syscalls/posix_fadvise0[13]: Start using new library.

 testcases/kernel/syscalls/chmod/chmod05.c     | 187 ++++--------------
 testcases/kernel/syscalls/chmod/chmod07.c     | 174 ++++------------
 testcases/kernel/syscalls/chroot/chroot01.c   |   4 +-
 .../kernel/syscalls/fadvise/posix_fadvise01.c | 135 +++----------
 .../kernel/syscalls/fadvise/posix_fadvise03.c | 163 ++++-----------
 testcases/kernel/syscalls/fchmod/fchmod02.c   |   7 +-
 6 files changed, 148 insertions(+), 522 deletions(-)

1] https://android-review.googlesource.com/c/platform/external/ltp/+/813818
https://android-review.googlesource.com/c/platform/external/ltp/+/813822

Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#1 lore raw

PATCH 1/5 chroot01: Use the 'tmpdir' we create.

Remove hard coded use of /tmp that breaks this test for Android and use the tmpdir() we create in the setup() instead.

---
 testcases/kernel/syscalls/chroot/chroot01.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
testcases/kernel/syscalls/chroot/chroot01.c +3 -1
index 9b0c30cf1..a1db5e157 100644
@@ -54,7 +54,7 @@ char *TCID = "chroot01";
 int TST_TOTAL = 1;
 int fail;
 
-char path[] = "/tmp";
+char *path;
 
 char nobody_uid[] = "nobody";
 struct passwd *ltpuser;
@@ -94,6 +94,7 @@ void setup(void)
 	tst_require_root();
 
 	tst_tmpdir();
+	path = tst_get_tmpdir();
 
 	if ((ltpuser = getpwnam(nobody_uid)) == NULL)
 		tst_brkm(TBROK | TERRNO, cleanup,
@@ -110,5 +111,6 @@ void cleanup(void)
 {
 	SAFE_SETEUID(NULL, 0);
 
+	free(path);
 	tst_rmdir();
 }
Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#3 lore raw

PATCH 2/5 syscalls/chmod07: Use new ltp library

In reply to Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code. Side benefit of making sure the test runs on Android when using SAFE_GETGRNAM_FALLBACK()

---
 testcases/kernel/syscalls/chmod/chmod07.c | 174 ++++++----------------
 1 file changed, 42 insertions(+), 132 deletions(-)
testcases/kernel/syscalls/chmod/chmod07.c +42 -132
index 6a3938840..4e43f2e32 100644
@@ -1,20 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- *
  *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 /*
@@ -31,43 +17,6 @@
  * Expected Result:
  *  chmod() should return value 0 on success and succeeds to set sticky bit
  *  on the specified file.
- *
- * 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>
- *  chmod07 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
- *     where,  -c n : Run n copies concurrently.
- *             -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:
- *  This test should be run by 'super-user' (root) only.
- *
  */
 
 #include <stdio.h>
@@ -80,75 +29,46 @@
 #include <grp.h>
 #include <pwd.h>
 
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
-#define LTPUSER		"nobody"
-#define LTPGRP		"users"
-#define FILE_MODE 	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
-#define PERMS		01777	/*
-				 * Mode permissions of test file with sticky
-				 * bit set.
-				 */
+#define FILE_MODE 	(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
+#define PERMS		01777	/* Permissions with sticky bit set. */
 #define TESTFILE	"testfile"
 
-char *TCID = "chmod07";
-int TST_TOTAL = 1;
-
-void setup();			/* Main setup function for the test */
-void cleanup();			/* Main cleanup function for the test */
-
-int main(int ac, char **av)
+void test_chmod(void)
 {
 	struct stat stat_buf;	/* stat(2) struct contents */
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
 
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/*
-		 * Call chmod(2) with specified mode argument
-		 * (sticky-bit set) on testfile.
-		 */
-		TEST(chmod(TESTFILE, PERMS));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "chmod(%s, %#o) failed",
-				 TESTFILE, PERMS);
-			continue;
-		}
-		/*
-		 * Get the testfile information using
-		 * stat(2).
-		 */
-		if (stat(TESTFILE, &stat_buf) == -1)
-			tst_brkm(TFAIL | TTERRNO, cleanup,
-				 "stat failed");
+	/*
+	 * Call chmod(2) with specified mode argument
+	 * (sticky-bit set) on testfile.
+	 */
+	TEST(chmod(TESTFILE, PERMS));
+	if (TST_RET == -1) {
+		tst_brk(TFAIL | TTERRNO, "chmod(%s, %#o) failed",
+				TESTFILE, PERMS);
+	}
 
-		/* Check for expected mode permissions */
-		if ((stat_buf.st_mode & PERMS) == PERMS)
-			tst_resm(TPASS, "Functionality of "
-				 "chmod(%s, %#o) successful",
-				 TESTFILE, PERMS);
-		else
-			tst_resm(TFAIL, "%s: Incorrect modes 0%03o; "
-				 "expected 0%03o", TESTFILE,
-				 stat_buf.st_mode, PERMS);
+	/*
+	 * Get the testfile information using
+	 * stat(2).
+	 */
+	if (stat(TESTFILE, &stat_buf) == -1) {
+		tst_brk(TFAIL | TTERRNO, "stat failed");
 	}
 
-	cleanup();
-	tst_exit();
+	/* Check for expected mode permissions */
+	if ((stat_buf.st_mode & PERMS) == PERMS) {
+		tst_res(TPASS, "Functionality of chmod(%s, %#o) successful",
+				TESTFILE, PERMS);
+	} else {
+		tst_res(TFAIL, "%s: Incorrect modes 0%03o; expected 0%03o",
+				TESTFILE, stat_buf.st_mode, PERMS);
+	}
 }
 
 /*
- * void
- * setup() - performs all ONE TIME setup for this test.
- *  Create a temporary directory and change directory to it.
+ *  setup() - performs all ONE TIME setup for this test.
  *  Create a test file under temporary directory and close it
  *  Change the ownership of test file to that of "ltpuser1" user.
  */
@@ -160,32 +80,22 @@ void setup(void)
 	gid_t group1_gid;	/* user and process group id's */
 	uid_t user1_uid;
 
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	tst_require_root();
-
-	tst_tmpdir();
-
-	/* Get the uid of guest user - ltpuser1 */
-	if ((ltpuser = getpwnam(LTPUSER)) == NULL)
-		tst_brkm(TBROK, cleanup, "getpwnam failed");
+	/* Get the uid of guest user - nobody */
+	ltpuser = SAFE_GETPWNAM("nobody");
 	user1_uid = ltpuser->pw_uid;
 
-	/* Get the group id of guest user - ltpuser1 */
-	if ((ltpgroup = getgrnam(LTPGRP)) == NULL)
-		tst_brkm(TBROK, cleanup, "getgrnam failed");
+	ltpgroup = SAFE_GETGRNAM_FALLBACK("users", "daemon");
 	group1_gid = ltpgroup->gr_gid;
 
-	fd = SAFE_OPEN(cleanup, TESTFILE, O_RDWR | O_CREAT, FILE_MODE);
-	SAFE_CLOSE(cleanup, fd);
-	SAFE_CHOWN(cleanup, TESTFILE, user1_uid, group1_gid);
-
-	SAFE_SETGID(cleanup, group1_gid);
+	fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, FILE_MODE);
+	SAFE_CLOSE(fd);
+	SAFE_CHOWN(TESTFILE, user1_uid, group1_gid);
+	SAFE_SETGID(group1_gid);
 }
 
-void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.test_all = test_chmod,
+};
Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#4 lore raw

PATCH 2/5 syscalls/chmod07: Use new ltp library

Hi!

testcases/kernel/syscalls/chmod/chmod07.c line 1

Quoted 10 lines
---
 testcases/kernel/syscalls/chmod/chmod07.c | 174 ++++++----------------
 1 file changed, 42 insertions(+), 132 deletions(-)
testcases/kernel/syscalls/chmod/chmod07.c +1 -0
index 6a3938840..4e43f2e32 100644
@@ -1,20 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0

Should have been GPL-2.0-or-later

I fixed that and also removed a few useless comments in the source and pushed, thanks.

Signature · 2 lines
Cyril Hrubis
[email protected]
#5 lore raw

PATCH 3/5 syscalls/fchmod02: fall back to use "daemon" group if "users" is absent.

In reply to Sandeep Patil

This makes the test work successfully on Android systems where "users" group doesn't exist.

---
 testcases/kernel/syscalls/fchmod/fchmod02.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
testcases/kernel/syscalls/fchmod/fchmod02.c +2 -5
index d0b35db12..e60cb33a6 100644
@@ -22,9 +22,6 @@
 #include "tst_test.h"
 #include "fchmod.h"
 
-#define LTPUSER		"nobody"
-#define LTPGRP		"users"
-
 static int fd;
 
 static void verify_fchmod(void)
@@ -53,8 +50,8 @@ static void setup(void)
 	struct passwd *ltpuser;
 	struct group *ltpgroup;
 
-	ltpuser = SAFE_GETPWNAM(LTPUSER);
-	ltpgroup = SAFE_GETGRNAM(LTPGRP);
+	ltpuser = SAFE_GETPWNAM("nobody");
+	ltpgroup = SAFE_GETGRNAM_FALLBACK("users", "daemon");
 
 	fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, FILE_MODE);
 	SAFE_CHOWN(TESTFILE, ltpuser->pw_uid, ltpgroup->gr_gid);
Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#6 lore raw

PATCH 3/5 syscalls/fchmod02: fall back to use "daemon" group if "users" is absent.

Hi!
Pushed, thanks.

Signature · 2 lines
Cyril Hrubis
[email protected]
#7 lore raw

PATCH 4/5 syscalls/chmod05: Use new ltp library

In reply to Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code while at it.

---
 testcases/kernel/syscalls/chmod/chmod05.c | 187 ++++------------------
 1 file changed, 33 insertions(+), 154 deletions(-)
testcases/kernel/syscalls/chmod/chmod05.c +33 -154
index 3cf4db5d0..c18a4778a 100644
@@ -1,30 +1,12 @@
+/// SPDX-License-Identifier: GPL-2.0
 /*
- *
  *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 /*
  * Test Name: chmod05
  *
  * Test Description:
-//wjh
-//This test seems to be invalid see comments below
-//The man page for chmod doesn't seem to indicate that the setgid bit can
-//only be set by root
  *  Verify that, chmod(2) will succeed to change the mode of a directory
  *  but fails to set the setgid bit on it if invoked by non-root (uid != 0)
  *  process with the following constraints,
@@ -36,48 +18,6 @@
  *  chmod() should return value 0 on success and though succeeds to change
  *  the mode of a directory but fails to set setgid bit on it.
  *
- * 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>
- *  chmod05 [-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
- *	05/2002 Fixes by wjhuie and Ferhan Shareef
- *                  changed conditional to check for valid tests only and
- *                  in a more logically understandable way
- *	07/2002 Additional fixes to #defines to allow comparisions to work.
- *		-Robbie Williamson
- *
- * RESTRICTIONS:
- *  This test should be run by root.
- *
  */
 
 #ifndef _GNU_SOURCE
@@ -96,126 +36,65 @@
 #include <grp.h>
 #include <pwd.h>
 
-#include "test.h"
-#include "safe_macros.h"
-
-#define DEBUG 0
+#include "tst_test.h"
 
 #define MODE_RWX	(mode_t)(S_IRWXU | S_IRWXG | S_IRWXO)
 #define DIR_MODE	(mode_t)(S_ISVTX | S_ISGID | S_IFDIR)
 #define PERMS		(mode_t)(MODE_RWX | DIR_MODE)
 #define TESTDIR		"testdir"
 
-char *TCID = "chmod05";
-int TST_TOTAL = 1;
-
-void setup();			/* Main setup function for test */
-void cleanup();			/* Main cleanup function for test */
-
-int main(int ac, char **av)
+void test_chmod(void)
 {
 	struct stat stat_buf;	/* stat struct */
-	int lc;
 	mode_t dir_mode;	/* mode permissions set on test directory */
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		TEST(chmod(TESTDIR, PERMS));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL, "chmod(%s, %#o) failed",
-				 TESTDIR, PERMS);
-			continue;
-		}
-		/*
-		 * Get the directory information using
-		 * stat(2).
-		 */
-		if (stat(TESTDIR, &stat_buf) < 0) {
-			tst_brkm(TFAIL | TERRNO, cleanup,
-				 "stat(%s) failed", TESTDIR);
-		}
-		dir_mode = stat_buf.st_mode;
-#if DEBUG
-		printf("DIR_MODE = 0%03o\n", DIR_MODE);
-		printf("MODE_RWX = 0%03o\n", MODE_RWX);
-		printf("PERMS = 0%03o\n", PERMS);
-		printf("dir_mode = 0%03o\n", dir_mode);
-#endif
-		if ((PERMS & ~S_ISGID) != dir_mode)
-			tst_resm(TFAIL, "%s: Incorrect modes 0%03o, "
-				 "Expected 0%03o", TESTDIR, dir_mode,
-				 PERMS & ~S_ISGID);
-		else
-			tst_resm(TPASS,
-				 "Functionality of chmod(%s, %#o) successful",
-					 TESTDIR, PERMS);
+	TEST(chmod(TESTDIR, PERMS));
+	if (TST_RET == -1) {
+		tst_res(TFAIL, "chmod(%s, %#o) failed", TESTDIR, PERMS);
 	}
 
-	cleanup();
-	tst_exit();
+	/* Get the directory information using stat(2) and compare with
+	 * expected results.
+	 */
+	SAFE_STAT(TESTDIR, &stat_buf);
+	dir_mode = stat_buf.st_mode;
+	if ((PERMS & ~S_ISGID) != dir_mode) {
+		tst_res(TFAIL, "%s: Incorrect modes 0%03o, "
+				"Expected 0%03o", TESTDIR, dir_mode,
+				PERMS & ~S_ISGID);
+	} else {
+		tst_res(TPASS, "Functionality of chmod(%s, %#o) successful",
+				TESTDIR, PERMS);
+	}
 }
 
-/*
- * void
- * setup() - performs all ONE TIME setup for this test.
- *  Create a temporary directory and cd to it.
- *  Create a test directory under temporary directory.
-//wjh we are root so do we really need this kluged helper program?
- *  Invoke setuid to root program to modify group ownership
- *  on test directory.
- */
 void setup(void)
 {
 	struct passwd *nobody_u;
-	struct group *bin_group;
-
-	tst_require_root();
+	struct group *bin_gr;
 
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	nobody_u = getpwnam("nobody");
-	if (nobody_u == NULL)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "getpwnam(\"nobody\") failed");
-
-	bin_group = getgrnam("bin");
-	if (bin_group == NULL)
-		tst_brkm(TBROK | TERRNO, cleanup, "getgrnam(\"bin\") failed");
+	nobody_u = SAFE_GETPWNAM("nobody");
+	bin_gr = SAFE_GETGRNAM("bin");
 
 	/*
 	 * Create a test directory under temporary directory with specified
 	 * mode permissions and change the gid of test directory to nobody's
 	 * gid.
 	 */
-	SAFE_MKDIR(cleanup, TESTDIR, MODE_RWX);
-
+	SAFE_MKDIR(TESTDIR, MODE_RWX);
 	if (setgroups(1, &nobody_u->pw_gid) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "setgroups to nobody's gid failed");
+		tst_brk(TBROK | TERRNO, "setgroups to nobody's gid failed");
 
-	SAFE_CHOWN(cleanup, TESTDIR, nobody_u->pw_uid, bin_group->gr_gid);
+	SAFE_CHOWN(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid);
 
 	/* change to nobody:nobody */
-	if (setegid(nobody_u->pw_gid) == -1 || seteuid(nobody_u->pw_uid) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "Couldn't switch to nobody:nobody");
+	SAGE_SETEGID(nobody_u->pw_gid);
+	SAFE_SETEUID(nobody_u->pw_uid);
 }
 
-void cleanup(void)
-{
-	if (setegid(0) == -1)
-		tst_resm(TWARN | TERRNO, "setegid(0) failed");
-	if (seteuid(0) == -1)
-		tst_resm(TWARN | TERRNO, "seteuid(0) failed");
-
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.test_all = test_chmod,
+};
Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#8 lore raw

PATCH 4/5 syscalls/chmod05: Use new ltp library

Quoted 233 lines · Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code while at it.

---
 testcases/kernel/syscalls/chmod/chmod05.c | 187 ++++------------------
 1 file changed, 33 insertions(+), 154 deletions(-)
testcases/kernel/syscalls/chmod/chmod05.c +26 -145
index 3cf4db5d0..c18a4778a 100644
@@ -1,30 +1,12 @@
+/// SPDX-License-Identifier: GPL-2.0
 /*
- *
  *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 /*
  * Test Name: chmod05
  *
  * Test Description:
-//wjh
-//This test seems to be invalid see comments below
-//The man page for chmod doesn't seem to indicate that the setgid bit can
-//only be set by root
  *  Verify that, chmod(2) will succeed to change the mode of a directory
  *  but fails to set the setgid bit on it if invoked by non-root (uid != 0)
  *  process with the following constraints,
@@ -36,48 +18,6 @@
  *  chmod() should return value 0 on success and though succeeds to change
  *  the mode of a directory but fails to set setgid bit on it.
  *
- * 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>
- *  chmod05 [-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
- *	05/2002 Fixes by wjhuie and Ferhan Shareef
- *                  changed conditional to check for valid tests only and
- *                  in a more logically understandable way
- *	07/2002 Additional fixes to #defines to allow comparisions to work.
- *		-Robbie Williamson
- *
- * RESTRICTIONS:
- *  This test should be run by root.
- *
  */
 
 #ifndef _GNU_SOURCE
@@ -96,126 +36,65 @@
 #include <grp.h>
 #include <pwd.h>
 
-#include "test.h"
-#include "safe_macros.h"
-
-#define DEBUG 0
+#include "tst_test.h"
 
 #define MODE_RWX	(mode_t)(S_IRWXU | S_IRWXG | S_IRWXO)
 #define DIR_MODE	(mode_t)(S_ISVTX | S_ISGID | S_IFDIR)
 #define PERMS		(mode_t)(MODE_RWX | DIR_MODE)
 #define TESTDIR		"testdir"
 
-char *TCID = "chmod05";
-int TST_TOTAL = 1;
-
-void setup();			/* Main setup function for test */
-void cleanup();			/* Main cleanup function for test */
-
-int main(int ac, char **av)
+void test_chmod(void)
 {
 	struct stat stat_buf;	/* stat struct */
-	int lc;
 	mode_t dir_mode;	/* mode permissions set on test directory */
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		TEST(chmod(TESTDIR, PERMS));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL, "chmod(%s, %#o) failed",
-				 TESTDIR, PERMS);
-			continue;
-		}
-		/*
-		 * Get the directory information using
-		 * stat(2).
-		 */
-		if (stat(TESTDIR, &stat_buf) < 0) {
-			tst_brkm(TFAIL | TERRNO, cleanup,
-				 "stat(%s) failed", TESTDIR);
-		}
-		dir_mode = stat_buf.st_mode;
-#if DEBUG
-		printf("DIR_MODE = 0%03o\n", DIR_MODE);
-		printf("MODE_RWX = 0%03o\n", MODE_RWX);
-		printf("PERMS = 0%03o\n", PERMS);
-		printf("dir_mode = 0%03o\n", dir_mode);
-#endif
-		if ((PERMS & ~S_ISGID) != dir_mode)
-			tst_resm(TFAIL, "%s: Incorrect modes 0%03o, "
-				 "Expected 0%03o", TESTDIR, dir_mode,
-				 PERMS & ~S_ISGID);
-		else
-			tst_resm(TPASS,
-				 "Functionality of chmod(%s, %#o) successful",
-					 TESTDIR, PERMS);
+	TEST(chmod(TESTDIR, PERMS));
+	if (TST_RET == -1) {
+		tst_res(TFAIL, "chmod(%s, %#o) failed", TESTDIR, PERMS);
 	}
 
-	cleanup();
-	tst_exit();
+	/* Get the directory information using stat(2) and compare with
+	 * expected results.
+	 */
+	SAFE_STAT(TESTDIR, &stat_buf);
+	dir_mode = stat_buf.st_mode;
+	if ((PERMS & ~S_ISGID) != dir_mode) {
+		tst_res(TFAIL, "%s: Incorrect modes 0%03o, "
+				"Expected 0%03o", TESTDIR, dir_mode,
+				PERMS & ~S_ISGID);
+	} else {
+		tst_res(TPASS, "Functionality of chmod(%s, %#o) successful",
+				TESTDIR, PERMS);
+	}
 }
 
-/*
- * void
- * setup() - performs all ONE TIME setup for this test.
- *  Create a temporary directory and cd to it.
- *  Create a test directory under temporary directory.
-//wjh we are root so do we really need this kluged helper program?
- *  Invoke setuid to root program to modify group ownership
- *  on test directory.
- */
 void setup(void)
 {
 	struct passwd *nobody_u;
-	struct group *bin_group;
-
-	tst_require_root();
+	struct group *bin_gr;
 
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	nobody_u = getpwnam("nobody");
-	if (nobody_u == NULL)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "getpwnam(\"nobody\") failed");
-
-	bin_group = getgrnam("bin");
-	if (bin_group == NULL)
-		tst_brkm(TBROK | TERRNO, cleanup, "getgrnam(\"bin\") failed");
+	nobody_u = SAFE_GETPWNAM("nobody");
+	bin_gr = SAFE_GETGRNAM("bin");
 
 	/*
 	 * Create a test directory under temporary directory with specified
 	 * mode permissions and change the gid of test directory to nobody's
 	 * gid.
 	 */
-	SAFE_MKDIR(cleanup, TESTDIR, MODE_RWX);
-
+	SAFE_MKDIR(TESTDIR, MODE_RWX);
 	if (setgroups(1, &nobody_u->pw_gid) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "setgroups to nobody's gid failed");
+		tst_brk(TBROK | TERRNO, "setgroups to nobody's gid failed");
 
-	SAFE_CHOWN(cleanup, TESTDIR, nobody_u->pw_uid, bin_group->gr_gid);
+	SAFE_CHOWN(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid);
 
 	/* change to nobody:nobody */
-	if (setegid(nobody_u->pw_gid) == -1 || seteuid(nobody_u->pw_uid) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup,
-			 "Couldn't switch to nobody:nobody");
+	SAGE_SETEGID(nobody_u->pw_gid);

testcases/kernel/syscalls/chmod/chmod05.c line 219

Quoted 10 lines
testcases/kernel/syscalls/chmod/chmod05.c +1 -7
+	SAFE_SETEUID(nobody_u->pw_uid);
 }
 
-void cleanup(void)
-{
-	if (setegid(0) == -1)
-		tst_resm(TWARN | TERRNO, "setegid(0) failed");
-	if (seteuid(0) == -1)
-		tst_resm(TWARN | TERRNO, "seteuid(0) failed");
-

It looks like I need to do this in some cleanup function for the new test. Otherwise the library cleanup won't be able to delete the tmpdir. Let me rework this..

- ssp

Quoted 11 lines
testcases/kernel/syscalls/chmod/chmod05.c +6 -2
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.test_all = test_chmod,
+};
Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#9 lore raw

PATCH 4/5 syscalls/chmod05: Use new ltp library

Hi!

Quoted 11 lines

testcases/kernel/syscalls/chmod/chmod05.c line 219

Quoted 7 lines
testcases/kernel/syscalls/chmod/chmod05.c +0 -7
-void cleanup(void)
-{
-	if (setegid(0) == -1)
-		tst_resm(TWARN | TERRNO, "setegid(0) failed");
-	if (seteuid(0) == -1)
-		tst_resm(TWARN | TERRNO, "seteuid(0) failed");
-

It looks like I need to do this in some cleanup function for the new test. Otherwise the library cleanup won't be able to delete the tmpdir. Let me rework this..

Actually the temporary directory cleanup in the new library is executed from the parent process, the actuall test is executed from a forked child so it shouldn't matter what euid and egid the child process ends up with.

Signature · 2 lines
Cyril Hrubis
[email protected]
#10 lore raw

PATCH 4/5 syscalls/chmod05: Use new ltp library

Quoted 13 lines · Cyril Hrubis

Hi!

Quoted 11 lines

testcases/kernel/syscalls/chmod/chmod05.c line 219

Quoted 7 lines
testcases/kernel/syscalls/chmod/chmod05.c +0 -7
-void cleanup(void)
-{
-	if (setegid(0) == -1)
-		tst_resm(TWARN | TERRNO, "setegid(0) failed");
-	if (seteuid(0) == -1)
-		tst_resm(TWARN | TERRNO, "seteuid(0) failed");
-

It looks like I need to do this in some cleanup function for the new test. Otherwise the library cleanup won't be able to delete the tmpdir. Let me rework this..

Quoted 4 lines

Actually the temporary directory cleanup in the new library is executed from the parent process, the actuall test is executed from a forked child so it shouldn't matter what euid and egid the child process ends up with.

Well too late, I added it again :). Anyway, this patch suffered from build errors too. Turns out 'make install' doesn't build things again so I missed them. I have fixed them up now and reset, feel free to delete the cleanup function I added there (will respond there too)

-ssp

Quoted 9 lines
Signature · 7 lines
-- 
Cyril Hrubis
[email protected]

-- 
You received this message because you are subscribed to the Google Groups "kernel-team" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
#11 lore raw

PATCH 5/5 syscalls/posix_fadvise0[13]: Start using new library.

In reply to Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code while at it. Make sure the tests can work on any system by creating its own file and using it for testing.

---
 .../kernel/syscalls/fadvise/posix_fadvise01.c | 135 ++++-----------
 .../kernel/syscalls/fadvise/posix_fadvise03.c | 163 ++++--------------
 2 files changed, 68 insertions(+), 230 deletions(-)
testcases/kernel/syscalls/fadvise/posix_fadvise01.c +31 -104
index c12f0563c..ad139a66b 100644
@@ -1,20 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- *
  *   Copyright (c) Red Hat Inc., 2007
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 /*
@@ -34,15 +20,13 @@
  *	None
  */
 
-#define _XOPEN_SOURCE 600
 #include <fcntl.h>
 
 #include <unistd.h>
 #include <signal.h>
 #include <errno.h>
 
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 
 #include "lapi/syscalls.h"
 #ifndef _FILE_OFFSET_BITS
@@ -53,15 +37,8 @@
 #define __NR_fadvise64 0
 #endif
 
-void setup();
-void cleanup();
-
-TCID_DEFINE(posix_fadvise01);
-
-char fname[] = "/bin/cat";	/* test executable to open */
-int fd = -1;			/* initialized in open */
-
-int expected_return = 0;
+const char *fname = "testfile";
+int fd = -1;
 
 int defined_advise[] = {
 	POSIX_FADV_NORMAL,
@@ -72,91 +49,41 @@ int defined_advise[] = {
 	POSIX_FADV_DONTNEED,
 };
 
-#define defined_advise_total ARRAY_SIZE(defined_advise)
-
-int TST_TOTAL = defined_advise_total;
-
-int main(int ac, char **av)
+void test_posix_fadvise(unsigned int nr)
 {
-	int lc;
-	int i;
-
-	/* Check this system has fadvise64 system which is used
-	   in posix_fadvise. */
-	if ((_FILE_OFFSET_BITS != 64) && (__NR_fadvise64 == 0)) {
-		tst_resm(TWARN,
-			 "This test can only run on kernels that implements ");
-		tst_resm(TWARN, "fadvise64 which is used from posix_fadvise");
-		exit(0);
+	TEST(posix_fadvise(fd, 0, 0, defined_advise[nr]));
+	if (TST_RET == 0) {
+		tst_res(TPASS, "call succeeded expectedly");
+	} else {
+		tst_res(TFAIL, "unexpected return - %ld w/ advise %d",
+				TST_RET, defined_advise[nr]);
 	}
-
-	/*
-	 * parse standard options
-	 */
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	/*
-	 * perform global setup for test
-	 */
-	setup();
-
-	/*
-	 * check looping state if -i option given on the command line
-	 */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		/* loop through the test cases */
-		for (i = 0; i < defined_advise_total; i++) {
-
-			TEST(posix_fadvise(fd, 0, 0, defined_advise[i]));
-
-			/* Man page says:
-			   "On error, an error number is returned." */
-			if (TEST_RETURN == expected_return) {
-				tst_resm(TPASS, "call succeeded expectedly");
-			} else {
-				tst_resm(TFAIL,
-					 "unexpected return value - %ld : %s, advise %d - "
-					 "expected %d",
-					 TEST_RETURN,
-					 strerror(TEST_RETURN),
-					 defined_advise[i], expected_return);
-			}
-		}
-	}
-
-	/*
-	 * cleanup and exit
-	 */
-	cleanup();
-
-	tst_exit();
 }
 
-/*
- * setup() - performs all ONE TIME setup for this test.
- */
 void setup(void)
 {
+	unsigned long pagesz = getpagesize();
+	char buf[10 * pagesz];
 
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	/* Check this system has fadvise64 system which is used
+	   in posix_fadvise. */
+	if ((_FILE_OFFSET_BITS != 64) && (__NR_fadvise64 == 0)) {
+		tst_brk(TCONF,
+			"This test can only run on kernels that implements "
+			"fadvise64 which is used from posix_fadvise");
+	}
 
-	TEST_PAUSE;
+	/* create 10 x pagesize file to be used for fadvise checks */
+	fd = SAFE_CREAT(fname, 0644);
+	SAFE_WRITE(1, fd, buf, 10 * pagesz);
+	SAFE_CLOSE(fd);
 
-	fd = SAFE_OPEN(cleanup, fname, O_RDONLY);
+	fd = SAFE_OPEN(fname, O_RDONLY);
 }
 
-/*
- * cleanup() - performs all ONE TIME cleanup for this test at
- *		completion or premature exit.
- */
-void cleanup(void)
-{
-
-	if (fd != -1) {
-		close(fd);
-	}
-
-}
+struct tst_test test = {
+	.tcnt = ARRAY_SIZE(defined_advise),
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.test = test_posix_fadvise,
+};
testcases/kernel/syscalls/fadvise/posix_fadvise03.c +37 -126

210 lines not on this page · raw message

Signature · 1 line
2.19.1.930.g4563a0d9d0-goog
#12 lore raw

PATCH 5/5 syscalls/posix_fadvise0[13]: Start using new library.

Quoted 7 lines · Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code while at it. Make sure the tests can work on any system by creating its own file and using it for testing.

---

Hi Sandeep,

Nice work.
Please compare with https://github.com/linux-test-project/ltp/pull/401
to see if you missed anything which I got.
I found some minor issue by diff (see below).

testcases/kernel/syscalls/fadvise/posix_fadvise01.c line 1

Quoted 10 lines
 .../kernel/syscalls/fadvise/posix_fadvise01.c | 135 ++++-----------
 .../kernel/syscalls/fadvise/posix_fadvise03.c | 163 ++++--------------
 2 files changed, 68 insertions(+), 230 deletions(-)
testcases/kernel/syscalls/fadvise/posix_fadvise01.c +1 -0
index c12f0563c..ad139a66b 100644
@@ -1,20 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0

// SPDX-License-Identifier: GPL-2.0-or-later

Here and in other test as well.

...

testcases/kernel/syscalls/fadvise/posix_fadvise01.c

Quoted 11 lines
testcases/kernel/syscalls/fadvise/posix_fadvise01.c +6 -5
-void cleanup(void)
-{
-
-       if (fd != -1) {
-               close(fd);
+       /* Check this system has fadvise64 system which is used
+          in posix_fadvise. */
+       if ((_FILE_OFFSET_BITS != 64) && (__NR_fadvise64 == 0)) {
+               tst_brk(TCONF,
+                       "This test can only run on kernels that implements "
+                       "fadvise64 which is used from posix_fadvise");

That is a good change compared to original test, but when Jan Stancek asked me about this check in my readahead test:
https://marc.info/?l=linux-unionfs&m=153857388324517&w=2
I looked into it and couldn''t find a reason for the test at all.

posix_fadvise() is a documented API and the tests posix_fadvise0[14] utilize this API. What does it matter which syscall variant is used to implement the API?

The only thing that should matter in the context of this test is whether or not posix_fadvise() is supported by the kernel.
For some reason that is not clear to me (are we tinyfying the kernel syscall by syscall now?), the support for fadvise in the kernel can be configured out with CONFIG_ADVISE_SYSCALLS=n. So IMO, this test really needs a runtime check, not a build time check like above.

We can try POSIX_FADV_NORMAL in setup() and report TCONF if it fails.

Quoted 3 lines
        }

+ fd = SAFE_OPEN(, fname, O_RDONLY);

typo SAFE_OPEN(,

Thanks,
Amir.

#13 lore raw

PATCH 5/5 syscalls/posix_fadvise0[13]: Start using new library.

Quoted 6 lines · Amir Goldstein
Quoted 5 lines · Sandeep Patil

Use SPDX-Licence-Identifier and delete dead comments / code while at it. Make sure the tests can work on any system by creating its own file and using it for testing.

Quoted 8 lines
Quoted 2 lines

---

Hi Sandeep,

Nice work.
Please compare with https://github.com/linux-test-project/ltp/pull/401
to see if you missed anything which I got.

I see Cyril merged that already now, so I'm going to drop these from the patch series. Thanks for the heads up, I guess I need to watch the github pull requests too now.

Quoted 15 lines

I found some minor issue by diff (see below).

testcases/kernel/syscalls/fadvise/posix_fadvise01.c line 1

Quoted 10 lines
 .../kernel/syscalls/fadvise/posix_fadvise01.c | 135 ++++-----------
 .../kernel/syscalls/fadvise/posix_fadvise03.c | 163 ++++--------------
 2 files changed, 68 insertions(+), 230 deletions(-)
testcases/kernel/syscalls/fadvise/posix_fadvise01.c +1 -0
index c12f0563c..ad139a66b 100644
@@ -1,20 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0

// SPDX-License-Identifier: GPL-2.0-or-later

Quoted 1 line

Here and in other test as well.

Yeah, missed it

Quoted 20 lines

...

testcases/kernel/syscalls/fadvise/posix_fadvise01.c

Quoted 11 lines
testcases/kernel/syscalls/fadvise/posix_fadvise01.c +6 -5
-void cleanup(void)
-{
-
-       if (fd != -1) {
-               close(fd);
+       /* Check this system has fadvise64 system which is used
+          in posix_fadvise. */
+       if ((_FILE_OFFSET_BITS != 64) && (__NR_fadvise64 == 0)) {
+               tst_brk(TCONF,
+                       "This test can only run on kernels that implements "
+                       "fadvise64 which is used from posix_fadvise");

That is a good change compared to original test, but when Jan Stancek asked me about this check in my readahead test:
https://marc.info/?l=linux-unionfs&m=153857388324517&w=2
I looked into it and couldn''t find a reason for the test at all.

Quoted 3 lines

posix_fadvise() is a documented API and the tests posix_fadvise0[14] utilize this API. What does it matter which syscall variant is used to implement the API?

Agree, I honestly didn't look into the API and wanted to make sure I don't change the original behavior / code instead and then add changes to it later, anyway, I guess it doesn't matter anymore. Dropping the posix_fadvise() patches from this series now.

Quoted 9 lines

The only thing that should matter in the context of this test is whether or not posix_fadvise() is supported by the kernel.
For some reason that is not clear to me (are we tinyfying the kernel syscall by syscall now?), the support for fadvise in the kernel can be configured out with CONFIG_ADVISE_SYSCALLS=n. So IMO, this test really needs a runtime check, not a build time check like above.

We can try POSIX_FADV_NORMAL in setup() and report TCONF if it fails.

, I guess I'm going to drop these

Quoted 6 lines
Quoted 4 lines
        }

+ fd = SAFE_OPEN(, fname, O_RDONLY);

typo SAFE_OPEN(,

ew ... its scary that this didn't produce any errors :(.

Thanks for looking into this and showing me the pull request.

- ssp

Quoted 3 lines

Thanks,
Amir.

#14 lore raw

PATCH 5/5 syscalls/posix_fadvise0[13]: Start using new library.

In reply to Amir Goldstein

Hi!

Quoted 6 lines

The only thing that should matter in the context of this test is whether or not posix_fadvise() is supported by the kernel.
For some reason that is not clear to me (are we tinyfying the kernel syscall by syscall now?), the support for fadvise in the kernel can be configured out with CONFIG_ADVISE_SYSCALLS=n. So IMO, this test really needs a runtime check, not a build time check like above.

Yes, I would expect that we would get ENOSYS in a case that kernel is build without the support for the call. And we don't have to check in the setup, we can do tst_brk(TCONF, ...) in the actuall test as well.

Signature · 2 lines
Cyril Hrubis
[email protected]