Re: [LTP] [PATCH] setreuid04.c: Rewrite using new LTP API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Richard Palethorpe <rpalethorpe@suse.de> writes:

> Hello,
>
> Avinesh Kumar <akumar@suse.de> writes:
>
>> Signed-off-by: Avinesh Kumar <akumar@suse.de>
>> ---
>>  .../kernel/syscalls/setreuid/setreuid04.c     | 158 +++++-------------
>>  1 file changed, 41 insertions(+), 117 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setreuid/setreuid04.c b/testcases/kernel/syscalls/setreuid/setreuid04.c
>> index 8eed90df0..9c52ff1bd 100644
>> --- a/testcases/kernel/syscalls/setreuid/setreuid04.c
>> +++ b/testcases/kernel/syscalls/setreuid/setreuid04.c
>> @@ -1,141 +1,65 @@
>> +// 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
>> - *
>>   * Ported by John George
>> + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
>>   */
>>  
>> -/*
>> - * Test that root can change the real and effective uid to an
>> - * unpriviledged user.
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that root user can change the real and effective uid to an
>> + * unprivileged user.
>>   */
>>  
>> -#include <errno.h>
>> -#include <stdlib.h>
>>  #include <pwd.h>
>> -#include <sys/wait.h>
>> -
>> -#include "test.h"
>> -#include "compat_16.h"
>> -
>> -TCID_DEFINE(setreuid04);
>> +#include "tst_test.h"
>> +#include "compat_tst_16.h"
>>  
>>  static uid_t neg_one = -1;
>> +static uid_t root_uid, nobody_uid;
>>  
>> -static struct passwd nobody, root;
>> -
>> -/*
>> - * The following structure contains all test data.  Each structure in the array
>> - * is used for a separate test.  The tests are executed in the for loop below.
>> - */
>> -
>> -struct test_data_t {
>> +static struct tcase {
>>  	uid_t *real_uid;
>>  	uid_t *eff_uid;
>> -	struct passwd *exp_real_usr;
>> -	struct passwd *exp_eff_usr;
>> -	char *test_msg;
>> -} test_data[] = {
>> -	{
>> -	&neg_one, &neg_one, &root, &root, "After setreuid(-1, nobody),"}, {
>> -&nobody.pw_uid, &nobody.pw_uid, &nobody, &nobody,
>> -		    "After setreuid(-1, -1),"},};
>> -
>> -int TST_TOTAL = ARRAY_SIZE(test_data);
>> -
>> -static void setup(void);
>> -static void cleanup(void);
>> -static void uid_verify(struct passwd *, struct passwd *, char *);
>> -
>> -int main(int ac, char **av)
>> -{
>> -	int lc;
>> -
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		int i, pid;
>> -
>> -		tst_count = 0;
>> -
>> -		if ((pid = FORK_OR_VFORK()) == -1) {
>> -			tst_brkm(TBROK, cleanup, "fork failed");
>> -		} else if (pid == 0) {	/* child */
>> -
>> -			for (i = 0; i < TST_TOTAL; i++) {
>> -
>> -				/* Set the real or effective user id */
>> -				TEST(SETREUID(cleanup, *test_data[i].real_uid,
>> -					      *test_data[i].eff_uid));
>> -
>> -				if (TEST_RETURN != -1) {
>> -					tst_resm(TPASS, "setreuid(%d, %d) "
>> -						 "succeeded as expected.",
>> -						 *test_data[i].real_uid,
>> -						 *test_data[i].eff_uid);
>> -				} else {
>> -					tst_resm(TFAIL, "setreuid(%d, %d) "
>> -						 "did not return as expected.",
>> -						 *test_data[i].real_uid,
>> -						 *test_data[i].eff_uid);
>> -				}
>> -
>> -				uid_verify(test_data[i].exp_real_usr,
>> -					   test_data[i].exp_eff_usr,
>> -					   test_data[i].test_msg);
>> -			}
>> -			tst_exit();
>> -		} else {	/* parent */
>> -			tst_record_childstatus(cleanup, pid);
>> -		}
>> -	}
>> -	cleanup();
>> -	tst_exit();
>> -}
>> +	uid_t *exp_real_uid;
>> +	uid_t *exp_eff_uid;
>> +} tcases[] = {
>> +	{&neg_one, &neg_one, &root_uid, &root_uid},
>> +	{&nobody_uid, &nobody_uid, &nobody_uid, &nobody_uid}
>> +};
>
> I think further cleanup is possible here. We only have one test case so
> we don't need this struct or an array. We certainly don't need a struct
> of pointers. We don't need constants like "neg_one".

Setting to changes requested in patchwork due to the above

>
> -- 
> Thank you,
> Richard.


-- 
Thank you,
Richard.

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