[LTP] [PATCH] fchown/fchown04.c: cleanup

[LTP] [PATCH] fchown/fchown04.c: cleanup

[zenglg.jy]

cleanup of fchown04.c

Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/fchown/fchown04.c | 106 ++++++++--------------------
 1 file changed, 31 insertions(+), 75 deletions(-)

diff --git a/testcases/kernel/syscalls/fchown/fchown04.c b/testcases/kernel/syscalls/fchown/fchown04.c
index 8329550..707a979 100644
--- a/testcases/kernel/syscalls/fchown/fchown04.c
+++ b/testcases/kernel/syscalls/fchown/fchown04.c
@@ -13,8 +13,8 @@
  *   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
+ *   along with this program;  if not, write to the Free Software Foundation,
+ *   Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 /*
@@ -84,84 +84,59 @@
 #include "usctest.h"
 #include "safe_macros.h"
 
-#define MODE_RWX	(S_IRWXU|S_IRWXG|S_IRWXO)
-#define FILE_MODE	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
 #define TEST_FILE1	"tfile_1"
 #define TEST_FILE2	"tfile_2"
 
-void setup1();			/* setup function to test chmod for EPERM */
-void setup2();			/* setup function to test chmod for EBADF */
+static int fd1;
+static int fd2;
 
-int fd1;			/* File descriptor for testfile1 */
-int fd2;			/* File descriptor for testfile2 */
-
-struct test_case_t {		/* test case struct. to hold ref. test cond's */
-	int fd;
+static struct test_case_t {
+	int *fd;
 	int exp_errno;
-	void (*setupfunc) ();
 } test_cases[] = {
-	{
-	1, EPERM, setup1}, {
-2, EBADF, setup2},};
+	{&fd1, EPERM},
+	{&fd2, EBADF},
+};
 
-char test_home[PATH_MAX];	/* variable to hold TESTHOME env */
 char *TCID = "fchown04";
-int TST_TOTAL = 2;
-int exp_enos[] = { EPERM, EBADF, 0 };
-
-char bin_uid[] = "bin";
-struct passwd *ltpuser;
+int TST_TOTAL = ARRAY_SIZE(test_cases);
+static int exp_enos[] = { EPERM, EBADF, 0 };
 
-void setup();			/* Main setup function for the tests */
-void cleanup();			/* cleanup function for the test */
+static void setup(void);
+static void cleanup(void);
 
 int main(int ac, char **av)
 {
 	int lc;
-	char *msg;
-	int fd;			/* test file descriptor */
 	int i;
-	uid_t user_id;		/* Effective user id of a test process */
-	gid_t group_id;		/* Effective group id of a test process */
-
-	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
-		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
 
 	setup();
 
 	TEST_EXP_ENOS(exp_enos);
 
-	user_id = geteuid();
-	group_id = getegid();
-
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 
 		tst_count = 0;
 
 		for (i = 0; i < TST_TOTAL; i++) {
 
-			fd = test_cases[i].fd;
-
-			if (fd == 1)
-				fd = fd1;
-			else
-				fd = fd2;
-
-			TEST(fchown(fd, user_id, group_id));
+			TEST(fchown(*test_cases[i].fd, geteuid(), getegid()));
 
 			if (TEST_RETURN == -1) {
-				if (TEST_ERRNO == test_cases[i].exp_errno)
+				if (TEST_ERRNO == test_cases[i].exp_errno) {
 					tst_resm(TPASS | TTERRNO,
 						 "fchown failed as expected");
-				else
+				} else {
 					tst_resm(TFAIL | TTERRNO,
 						 "fchown failed unexpectedly; "
 						 "expected %d - %s",
 						 test_cases[i].exp_errno,
 						 strerror(test_cases[i].
 							  exp_errno));
-			} else
+				}
+			} else {
 				tst_resm(TFAIL, "fchown passed unexpectedly");
+			}
 		}
 
 	}
@@ -171,58 +146,39 @@ int main(int ac, char **av)
 	tst_exit();
 }
 
-void setup()
+static void setup(void)
 {
-	int i;
+	struct passwd *ltpuser;
 
 	TEST_PAUSE;
 
 	tst_require_root(NULL);
 
-	ltpuser = SAFE_GETPWNAM(NULL, bin_uid);
-
-	tst_tmpdir();
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	initgroups("root", getgid());
+	ltpuser = SAFE_GETPWNAM(cleanup, "nobody");
 
-	SAFE_SETEGID(NULL, ltpuser->pw_uid);
-	SAFE_SETEUID(NULL, ltpuser->pw_gid);
+	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
 
 	tst_tmpdir();
 
-	for (i = 0; i < TST_TOTAL; i++)
-		if (test_cases[i].setupfunc != NULL)
-			test_cases[i].setupfunc();
-}
-
-void setup1()
-{
-	int old_uid;
-	struct passwd *nobody;
+	tst_sig(FORK, DEF_HANDLER, cleanup);
 
+	/* EPERM */
 	fd1 = SAFE_OPEN(cleanup, TEST_FILE1, O_RDWR | O_CREAT, 0666);
 
-	old_uid = geteuid();
-
 	SAFE_SETEUID(cleanup, 0);
 
-	nobody = SAFE_GETPWNAM(cleanup, "nobody");
-
-	if (fchown(fd1, nobody->pw_uid, nobody->pw_gid) < 0)
+	if (fchown(fd1, 0, 0) < 0)
 		tst_brkm(TBROK | TERRNO, cleanup, "fchown failed");
 
-	SAFE_SETEUID(cleanup, old_uid);
-}
-
-void setup2()
-{
+	/* EBADF */
 	fd2 = SAFE_OPEN(cleanup, TEST_FILE2, O_RDWR | O_CREAT, 0666);
+
 	SAFE_CLOSE(cleanup, fd2);
+
+	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
 }
 
-void cleanup()
+static void cleanup(void)
 {
 	TEST_CLEANUP;
 
-- 
1.8.2.1




------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] fchown/fchown04.c: cleanup

[chrubis]

Hi!
> -char test_home[PATH_MAX];	/* variable to hold TESTHOME env */
>  char *TCID = "fchown04";
> -int TST_TOTAL = 2;
> -int exp_enos[] = { EPERM, EBADF, 0 };
> -
> -char bin_uid[] = "bin";
> -struct passwd *ltpuser;
> +int TST_TOTAL = ARRAY_SIZE(test_cases);
> +static int exp_enos[] = { EPERM, EBADF, 0 };
>  
> -void setup();			/* Main setup function for the tests */
> -void cleanup();			/* cleanup function for the test */
> +static void setup(void);
> +static void cleanup(void);
>  
>  int main(int ac, char **av)
>  {
>  	int lc;
> -	char *msg;
> -	int fd;			/* test file descriptor */
>  	int i;
> -	uid_t user_id;		/* Effective user id of a test process */
> -	gid_t group_id;		/* Effective group id of a test process */
> -
> -	if ((msg = parse_opts(ac, av, NULL, NULL)) != NULL)
> -		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);

Again, please do not remove the option parsing code.

>  	setup();
>  
>  	TEST_EXP_ENOS(exp_enos);
>  
> -	user_id = geteuid();
> -	group_id = getegid();
> -
>  	for (lc = 0; TEST_LOOPING(lc); lc++) {
>  
>  		tst_count = 0;
>  
>  		for (i = 0; i < TST_TOTAL; i++) {
>  
> -			fd = test_cases[i].fd;
> -
> -			if (fd == 1)
> -				fd = fd1;
> -			else
> -				fd = fd2;
> -
> -			TEST(fchown(fd, user_id, group_id));
> +			TEST(fchown(*test_cases[i].fd, geteuid(), getegid()));
>  
>  			if (TEST_RETURN == -1) {
> -				if (TEST_ERRNO == test_cases[i].exp_errno)
> +				if (TEST_ERRNO == test_cases[i].exp_errno) {
>  					tst_resm(TPASS | TTERRNO,
>  						 "fchown failed as expected");
> -				else
> +				} else {
>  					tst_resm(TFAIL | TTERRNO,
>  						 "fchown failed unexpectedly; "
>  						 "expected %d - %s",
>  						 test_cases[i].exp_errno,
>  						 strerror(test_cases[i].
>  							  exp_errno));
> -			} else
> +				}
> +			} else {
>  				tst_resm(TFAIL, "fchown passed unexpectedly");
> +			}
>  		}
>  
>  	}
> @@ -171,58 +146,39 @@ int main(int ac, char **av)
>  	tst_exit();
>  }
>  
> -void setup()
> +static void setup(void)
>  {
> -	int i;
> +	struct passwd *ltpuser;
>  
>  	TEST_PAUSE;
>  
>  	tst_require_root(NULL);
>  
> -	ltpuser = SAFE_GETPWNAM(NULL, bin_uid);
> -
> -	tst_tmpdir();
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	initgroups("root", getgid());
> +	ltpuser = SAFE_GETPWNAM(cleanup, "nobody");
>  
> -	SAFE_SETEGID(NULL, ltpuser->pw_uid);
> -	SAFE_SETEUID(NULL, ltpuser->pw_gid);
> +	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
>  
>  	tst_tmpdir();
>  
> -	for (i = 0; i < TST_TOTAL; i++)
> -		if (test_cases[i].setupfunc != NULL)
> -			test_cases[i].setupfunc();
> -}
> -
> -void setup1()
> -{
> -	int old_uid;
> -	struct passwd *nobody;
> +	tst_sig(FORK, DEF_HANDLER, cleanup);
>  
> +	/* EPERM */
>  	fd1 = SAFE_OPEN(cleanup, TEST_FILE1, O_RDWR | O_CREAT, 0666);
>  
> -	old_uid = geteuid();
> -
>  	SAFE_SETEUID(cleanup, 0);
>  
> -	nobody = SAFE_GETPWNAM(cleanup, "nobody");
> -
> -	if (fchown(fd1, nobody->pw_uid, nobody->pw_gid) < 0)
> +	if (fchown(fd1, 0, 0) < 0)
>  		tst_brkm(TBROK | TERRNO, cleanup, "fchown failed");
>  
> -	SAFE_SETEUID(cleanup, old_uid);
> -}
> -
> -void setup2()
> -{
> +	/* EBADF */
>  	fd2 = SAFE_OPEN(cleanup, TEST_FILE2, O_RDWR | O_CREAT, 0666);
> +
>  	SAFE_CLOSE(cleanup, fd2);
> +
> +	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
>  }
>  

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] fchown/fchown04.c: cleanup

[chrubis]

Hi!
> Cleanup of fchown04.c
>
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>

Pushed with following cosmetic changes:

-static int fd2;
+static int fd2 = -1;


		tst_count = 0;

-               for (i = 0; i < TST_TOTAL; i++) {
-
+               for (i = 0; i < TST_TOTAL; i++)
                        fchown_verify(i);
-               }
        }

-       fd2 = -1;
-


And with changes to cleanup to avoid SAFE_MACROS().

(See the the Test Writing Guidelines[1] chapter 2.2.4 Safe macros.)

[1] https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines

Thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2 2/2] fchown/fchown04.c: add EROFS error number test

[chrubis]

Hi!
> Add EROFS error number test for fchown(2)
> 
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
> ---
>  runtest/ltplite                             |  2 +-
>  runtest/stress.part3                        |  2 +-
>  runtest/syscalls                            |  4 +-
>  testcases/kernel/syscalls/fchown/fchown04.c | 57 ++++++++++++++++++++++++++++-
>  4 files changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/runtest/ltplite b/runtest/ltplite
> index c90bc48..c553746 100644
> --- a/runtest/ltplite
> +++ b/runtest/ltplite
> @@ -189,7 +189,7 @@ fchmod07 fchmod07
>  fchown01 fchown01
>  fchown02 fchown02
>  fchown03 cp -p $LTPROOT/testcases/bin/change_owner $TMPDIR;fchown03
> -fchown04 export change_owner=$LTPROOT/testcases/bin/change_owner;fchown04
> +fchown04 fchown04 -D DEVICE -T DEVICE_FS_TYPE
>  fchown05 fchown05

I've looked around the sources (git grep change_owner) and it looks like
none of the tests uses the change_owner anymore. Can you please send a
separate patch that removes the change_owner source code and cleans up
the referencies from runtest files and .gitignore?

Also please change the DEVICE and DEVICE_FS_TYPE to the new $LTP_DEV and
$LTP_DEV_FS_TYPE format.

> --- a/testcases/kernel/syscalls/fchown/fchown04.c
> +++ b/testcases/kernel/syscalls/fchown/fchown04.c
> @@ -25,6 +25,8 @@
>   *	not super user.
>   *   2) fchown(2) returns -1 and sets errno to EBADF if the file descriptor
>   *	of the specified file is not valid.
> + *   3) fchown(2) returns -1 and sets errno to EROFS if the named file resides
> + *	on a read-only file system.
>   */
>  
>  #include <stdio.h>
> @@ -38,14 +40,28 @@
>  #include <pwd.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <sys/mount.h>
>  
>  #include "test.h"
>  #include "usctest.h"
>  #include "safe_macros.h"
>  
> +#define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
> +			 S_IXGRP|S_IROTH|S_IXOTH)
>  
>  static int fd1;
>  static int fd2;
> +static int fd3;
> +static char *fstype = "ext2";
> +static char *device;
> +static int dflag;
> +static int mount_flag;
> +
> +static option_t options[] = {
> +	{"T:", NULL, &fstype},
> +	{"D:", &dflag, &device},
> +	{NULL, NULL, NULL}
> +};
>  
>  static struct test_case_t {
>  	int *fd;
> @@ -53,15 +69,17 @@ static struct test_case_t {
>  } test_cases[] = {
>  	{&fd1, EPERM},
>  	{&fd2, EBADF},
> +	{&fd3, EROFS},
>  };
>  
>  char *TCID = "fchown04";
>  int TST_TOTAL = ARRAY_SIZE(test_cases);
> -static int exp_enos[] = { EPERM, EBADF, 0 };
> +static int exp_enos[] = { EPERM, EBADF, EROFS, 0 };
>  
>  static void setup(void);
>  static void fchown_verify(int);
>  static void cleanup(void);
> +static void help(void);
>  
>  int main(int ac, char **av)
>  {
> @@ -69,10 +87,16 @@ int main(int ac, char **av)
>  	char *msg;
>  	int i;
>  
> -	msg = parse_opts(ac, av, NULL, NULL);
> +	msg = parse_opts(ac, av, options, help);
>  	if (msg != NULL)
>  		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
>  
> +	if (!dflag) {
> +		tst_brkm(TBROK, NULL,
> +			 "you must specify the device used for mounting with "
> +			 "-D option");
> +	}

There is no need for the dflag. The device is static and initialized to
NULL so you can do if (!device) instead.

>  	setup();
>  
>  	TEST_EXP_ENOS(exp_enos);
> @@ -107,6 +131,21 @@ static void setup(void)
>  
>  	fd2 = -1;
>  
> +	tst_mkfs(NULL, device, fstype, NULL);
> +	SAFE_MKDIR(cleanup, "mntpoint", DIR_MODE);
> +	if (mount(device, "mntpoint", fstype, 0, NULL) < 0) {
> +		tst_brkm(TBROK | TERRNO, cleanup,
> +			 "mount device:%s failed", device);
> +	}
> +	mount_flag = 1;
> +	SAFE_TOUCH(cleanup, "mntpoint/tfile_3", 0644, NULL);
> +	if (mount(device, "mntpoint", fstype,
> +		  MS_REMOUNT | MS_RDONLY, NULL) < 0) {
> +		tst_brkm(TBROK | TERRNO, cleanup,
> +			 "mount device:%s failed", device);
> +	}
> +	fd3 = SAFE_OPEN(cleanup, "mntpoint/tfile_3", O_RDONLY);
> +
>  	ltpuser = SAFE_GETPWNAM(cleanup, "nobody");
>  	SAFE_SETEUID(cleanup, ltpuser->pw_uid);
>  }
> @@ -137,5 +176,19 @@ static void cleanup(void)
>  
>  	SAFE_CLOSE(NULL, fd1);
>  
> +	SAFE_CLOSE(NULL, fd3);
> +
> +	if (mount_flag && umount("mntpoint") < 0) {
> +		tst_brkm(TBROK | TERRNO, NULL,
> +			 "umount device:%s failed", device);

I wonder if it's right to call tst_brkm() here. Do we really want to
exit the cleanup() if we fail to umount the the device?

Also the problem with tst_brkm() is that it sometimes exits the
cleanup() which itself is a reason for avoiding tst_brkm() in cleanup().
(Currently it depends on whether callback() was called from tst_brkm()
or whether we called callback() directly which is another reason why the
hack from tst_brk() should go away...)

> +	}
> +
>  	tst_rmdir();
>  }

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience.  Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v3 2/2] fchown/fchown04.c: add EROFS error number test

[chrubis]

Hi!
> Add EROFS error number test for fchown(2)

Both pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list