[LTP] [PATCH v2 0/2] Misc cleanup (new lib) and fixes for Android

[LTP] [PATCH v2 0/2] Misc cleanup (new lib) and fixes for Android

[Sandeep Patil]

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]

v2:
-----
- Drop posix_fadvise patch (done in pull request by Amir Goldsten)
- Drop patches for chroot01 and chmod07, already merged.
- *really test and build the series*


Sandeep Patil (2):
  syscalls/fchmod02: fall back to use "daemon" group if "users" is
    absent.
  syscalls/chmod05: Use new ltp library

 testcases/kernel/syscalls/chmod/chmod05.c   | 191 ++++----------------
 testcases/kernel/syscalls/fchmod/fchmod02.c |   7 +-
 2 files changed, 41 insertions(+), 157 deletions(-)

-- 
2.19.1.930.g4563a0d9d0-goog

[LTP] [PATCH 1/2] syscalls/fchmod02: fall back to use "daemon" group if "users" is absent.

[Sandeep Patil]

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

Signed-off-by: Sandeep Patil <sspatil@google.com>
---
 testcases/kernel/syscalls/fchmod/fchmod02.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/fchmod/fchmod02.c b/testcases/kernel/syscalls/fchmod/fchmod02.c
index d0b35db12..e60cb33a6 100644
--- a/testcases/kernel/syscalls/fchmod/fchmod02.c
+++ b/testcases/kernel/syscalls/fchmod/fchmod02.c
@@ -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);
-- 
2.19.1.930.g4563a0d9d0-goog

[LTP] [PATCH 1/2] syscalls/fchmod02: fall back to use "daemon" group if "users" is absent.

[Sandeep Patil]

On Tue, Nov 06, 2018 at 08:11:25AM -0800, Sandeep Patil wrote:
> This makes the test work successfully on Android systems where
> "users" group doesn't exist.
> 
> Signed-off-by: Sandeep Patil <sspatil@google.com>
> ---
>  testcases/kernel/syscalls/fchmod/fchmod02.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fchmod/fchmod02.c b/testcases/kernel/syscalls/fchmod/fchmod02.c
> index d0b35db12..e60cb33a6 100644
> --- a/testcases/kernel/syscalls/fchmod/fchmod02.c
> +++ b/testcases/kernel/syscalls/fchmod/fchmod02.c
> @@ -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);
> -- 
> 2.19.1.930.g4563a0d9d0-goog
> 

(Ignore, this has been merged now)

[LTP] [PATCH 2/2] syscalls/chmod05: Use new ltp library

[Sandeep Patil]

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

Signed-off-by: Sandeep Patil <sspatil@google.com>
---

v1->v2:
------
- Fix SPDX identifier to make it GPLv2 or later.
- Fix build errors (missed cause 'make install' doesn't build things again)
- Add cleanup function to reset the effective uid / gid.

 testcases/kernel/syscalls/chmod/chmod05.c | 191 +++++-----------------
 1 file changed, 39 insertions(+), 152 deletions(-)

diff --git a/testcases/kernel/syscalls/chmod/chmod05.c b/testcases/kernel/syscalls/chmod/chmod05.c
index 3cf4db5d0..0e86f0691 100644
--- a/testcases/kernel/syscalls/chmod/chmod05.c
+++ b/testcases/kernel/syscalls/chmod/chmod05.c
@@ -1,30 +1,12 @@
+/// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
  *   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,73 @@
 #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;
+	struct group *bin_gr;
 
-	tst_require_root();
-
-	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_gr->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");
+	SAFE_SETEGID(nobody_u->pw_gid);
+	SAFE_SETEUID(nobody_u->pw_uid);
 }
 
-void cleanup(void)
+void cleanup_chmod(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();
+	/* Reset the uid / gid to root */
+	SAFE_SETEGID(0);
+	SAFE_SETEUID(0);
 }
+
+static struct tst_test test = {
+	.needs_root 	= 1,
+	.needs_tmpdir 	= 1,
+	.setup 		= setup,
+	.test_all 	= test_chmod,
+	.cleanup 	= cleanup_chmod,
+};
-- 
2.19.1.930.g4563a0d9d0-goog

[LTP] [PATCH 2/2] syscalls/chmod05: Use new ltp library

[Sandeep Patil]

On Tue, Nov 06, 2018 at 08:11:26AM -0800, Sandeep Patil wrote:
> Use SPDX-Licence-Identifier and delete dead comments / code
> while at it.
> 
> Signed-off-by: Sandeep Patil <sspatil@google.com>
> ---
> 
> v1->v2:
> ------
> - Fix SPDX identifier to make it GPLv2 or later.
> - Fix build errors (missed cause 'make install' doesn't build things again)
> - Add cleanup function to reset the effective uid / gid.
> 
>  testcases/kernel/syscalls/chmod/chmod05.c | 191 +++++-----------------
>  1 file changed, 39 insertions(+), 152 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/chmod/chmod05.c b/testcases/kernel/syscalls/chmod/chmod05.c
> index 3cf4db5d0..0e86f0691 100644
> --- a/testcases/kernel/syscalls/chmod/chmod05.c
> +++ b/testcases/kernel/syscalls/chmod/chmod05.c
> @@ -1,30 +1,12 @@
> +/// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - *
>   *   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,73 @@
>  #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;
> +	struct group *bin_gr;
>  
> -	tst_require_root();
> -
> -	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_gr->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");
> +	SAFE_SETEGID(nobody_u->pw_gid);
> +	SAFE_SETEUID(nobody_u->pw_uid);
>  }
>  
> -void cleanup(void)
> +void cleanup_chmod(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();
> +	/* Reset the uid / gid to root */
> +	SAFE_SETEGID(0);
> +	SAFE_SETEUID(0);
>  }

This was added, but I don't think it hurts anything, if at all it resets what
the test does in the setup (even though its probably not needed)?. I'll leave
it upto you to keep / delete this.

- ssp

> +
> +static struct tst_test test = {
> +	.needs_root 	= 1,
> +	.needs_tmpdir 	= 1,
> +	.setup 		= setup,
> +	.test_all 	= test_chmod,
> +	.cleanup 	= cleanup_chmod,
> +};
> -- 
> 2.19.1.930.g4563a0d9d0-goog
> 

[LTP] [PATCH 2/2] syscalls/chmod05: Use new ltp library

[Cyril Hrubis]

Hi!
> This was added, but I don't think it hurts anything, if at all it resets what
> the test does in the setup (even though its probably not needed)?. I'll leave
> it upto you to keep / delete this.

I've removed the cleanup and did a few minor changes as well and pushed,
thanks.

* Deleted /* stat struct */ and other comments commenting obvious

* Added return; in the if (TST_RET == -1) statement, there is no point
  in checking the directory permissions if the call has failed

* Removed spaces before tabs in the tst_test structure


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/chmod05: Use new ltp library

[Sandeep Patil]

On Tue, Nov 06, 2018 at 05:33:05PM +0100, Cyril Hrubis wrote:
> Hi!
> > This was added, but I don't think it hurts anything, if at all it resets what
> > the test does in the setup (even though its probably not needed)?. I'll leave
> > it upto you to keep / delete this.
> 
> I've removed the cleanup and did a few minor changes as well and pushed,
> thanks.
> 
> * Deleted /* stat struct */ and other comments commenting obvious
> 
> * Added return; in the if (TST_RET == -1) statement, there is no point
>   in checking the directory permissions if the call has failed
> 
> * Removed spaces before tabs in the tst_test structure

That was done to align the fields, I will be more mindful next time.
Thanks for making the changes.

- ssp

> 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz