Re: [LTP] [PATCH 2/3] syscalls/setgroups02: Convert to new API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +	TEST(SETGROUPS(gidsetsize, groups_set));

TST_EXP_PASS(SETGROUPS(gidsetsize, groups_set)) ?

> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -		tst_count = 0;
> -
> -		/*
> -		 * Call setgroups() to set supplimentary group IDs of
> -		 * the calling super-user process to gid of TESTUSER.
> -		 */
> -		TEST(SETGROUPS(cleanup, gidsetsize, groups_list));
> +	if (TST_RET < 0) {
> +		tst_res(TFAIL, "setgroups(%d, groups_set) Failed, "
> +			"errno=%d : %s", gidsetsize,
> +			TST_ERR, strerror(TST_ERR));
> +	}
>  
> -		if (TEST_RETURN == -1) {
> -			tst_resm(TFAIL, "setgroups(%d, groups_list) Failed, "
> -				 "errno=%d : %s", gidsetsize, TEST_ERRNO,
> -				 strerror(TEST_ERRNO));
> -			continue;
> -		}
> +	GETGROUPS(gidsetsize, groups_get);

TST_EXP_VAL(GETGROUPS(gitsetsize, groups_get), 1) ?

Also I suppose that we should make sure that the groups_get[0] value is
not equal to the groups_set[0], expecially with -i 2

So maybe:
	groups_get[0] = ~groups_set[0];

> -		/*
> -		 * Call getgroups(2) to verify that
> -		 * setgroups(2) successfully set the
> -		 * supp. gids of TESTUSER.
> -		 */
> -		groups_list[0] = '\0';
> -		if (GETGROUPS(cleanup, gidsetsize, groups_list) < 0) {
> -			tst_brkm(TFAIL, cleanup, "getgroups() Fails, "
> -				 "error=%d", errno);
> -		}
> -		for (i = 0; i < NGROUPS; i++) {
> -			if (groups_list[i] == user_info->pw_gid) {
> -				tst_resm(TPASS,
> -					 "Functionality of setgroups"
> -					 "(%d, groups_list) successful",
> -					 gidsetsize);
> -				PASS_FLAG = 1;
> -			}
> -		}
> -		if (PASS_FLAG == 0) {
> -			tst_resm(TFAIL, "Supplimentary gid %d not set "
> -				 "for the process", user_info->pw_gid);
> -		}
> +	if (groups_get[0] == groups_set[0]) {
> +		tst_res(TPASS,
> +			"Functionality of setgroups"
> +			"(%d, groups_set) successful",
> +			gidsetsize);
>  	}

TST_EXP_EQ_LI(groups_get[0], groups_set[0]);

At least that will produce TFAIL results if they don't match, which is
missing here.

> -	cleanup();
> -	tst_exit();
>  }
>  
> -/*
> - * setup() - performs all ONE TIME setup for this test.
> - *
> - *  Make sure the test process uid is root.
> - *  Get the supplimentrary group id of test user from /etc/passwd file.
> - */
> -void setup(void)
> +static void setup(void)
>  {
> +	user_info = SAFE_GETPWNAM(TESTUSER);
>  
> -	tst_require_root();
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	/* Get the group id info. of TESTUSER from /etc/passwd */
> -	if ((user_info = getpwnam(TESTUSER)) == NULL) {
> -		tst_brkm(TFAIL, cleanup, "getpwnam(2) of %s Failed", TESTUSER);
> -	}
> +	GID16_CHECK(user_info->pw_gid, setgroups);
>  
> -	if (!GID_SIZE_CHECK(user_info->pw_gid)) {
> -		tst_brkm(TBROK,
> -			 cleanup,
> -			 "gid returned from getpwnam is too large for testing setgroups16");
> -	}
> -
> -	groups_list[0] = user_info->pw_gid;
> +	groups_set[0] = user_info->pw_gid;

Do we really need this? I suppose that any random value will do, e.g.

	groups_set[0] = 42;

is going to be as good as anything else.

>  }
>  
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *	       completion or premature exit.
> - */
> -void cleanup(void)
> -{
> -
> -}
> +static struct tst_test test = {
> +	.test_all = verify_setgroups,
> +	.setup = setup,
> +	.needs_root = 1,
> +};
> -- 
> 1.8.3.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp