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

[Richard Palethorpe <rpalethorpe@suse.de>]

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".

-- 
Thank you,
Richard.

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