public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH v1 0/2] Refactor getegid testing suite
@ 2023-08-31 10:42 Andrea Cervesato
  2023-08-31 10:42 ` [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API Andrea Cervesato
  2023-08-31 10:42 ` [LTP] [PATCH v1 2/2] Refactor getegid02 " Andrea Cervesato
  0 siblings, 2 replies; 11+ messages in thread
From: Andrea Cervesato @ 2023-08-31 10:42 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

The getegid testing suite has been refactored using new LTP API, optimising the
code by adding some other checks.

Andrea Cervesato (2):
  Refactor getegid01 using new LTP API
  Refactor getegid02 using new LTP API

 testcases/kernel/syscalls/getegid/getegid01.c | 93 ++++---------------
 testcases/kernel/syscalls/getegid/getegid02.c | 93 +++++--------------
 2 files changed, 39 insertions(+), 147 deletions(-)

-- 
2.35.3


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-08-31 10:42 [LTP] [PATCH v1 0/2] Refactor getegid testing suite Andrea Cervesato
@ 2023-08-31 10:42 ` Andrea Cervesato
  2023-08-31 13:32   ` Cyril Hrubis
  2023-08-31 10:42 ` [LTP] [PATCH v1 2/2] Refactor getegid02 " Andrea Cervesato
  1 sibling, 1 reply; 11+ messages in thread
From: Andrea Cervesato @ 2023-08-31 10:42 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

This new version also verifies that getegid() effectively returns the
right effective group ID by looking at /proc/self/status information.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/getegid/getegid01.c | 93 ++++---------------
 1 file changed, 18 insertions(+), 75 deletions(-)

diff --git a/testcases/kernel/syscalls/getegid/getegid01.c b/testcases/kernel/syscalls/getegid/getegid01.c
index 271fbb6b6..acce75aa6 100644
--- a/testcases/kernel/syscalls/getegid/getegid01.c
+++ b/testcases/kernel/syscalls/getegid/getegid01.c
@@ -1,87 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it is
- * free of the rightful claim of any third person regarding infringement
- * or the like.  Any license provided herein, whether implied or
- * otherwise, applies only to this software file.  Patent licenses, if
- * any, provided herein do not apply to combinations of this program with
- * other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Contact information: Silicon Graphics, Inc., 1600 Amphitheatre Pkwy,
- * Mountain View, CA  94043, or:
- *
- * http://www.sgi.com
- *
- * For further information regarding this notice, see:
- *
- * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
- *
+ *   William Roske, Dave Fenner
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- *    AUTHOR		: William Roske
- *    CO-PILOT		: Dave Fenner
+/*\
+ * [Description]
+ *
+ * This test checks if getegid() returns the effective group id.
  */
 
-#include <sys/types.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-
-#include "test.h"
-#include "compat_16.h"
-
-static void setup();
-static void cleanup();
+#include "tst_test.h"
+#include "compat_tst_16.h"
 
-TCID_DEFINE(getegid01);
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static void run(void)
 {
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		TEST(GETEGID(cleanup));
+	gid_t gid;
+	gid_t st_gid, st_egid;
 
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "getegid failed");
-			continue;	/* next loop for MTKERNEL */
-		}
+	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);
 
-		tst_resm(TPASS, "getegid returned %ld", TEST_RETURN);
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	GID16_CHECK((gid = GETEGID()), "getegid");
+	TST_EXP_EQ_LI(gid, st_egid);
 }
 
-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.test_all = run,
+};
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v1 2/2] Refactor getegid02 using new LTP API
  2023-08-31 10:42 [LTP] [PATCH v1 0/2] Refactor getegid testing suite Andrea Cervesato
  2023-08-31 10:42 ` [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API Andrea Cervesato
@ 2023-08-31 10:42 ` Andrea Cervesato
  2023-09-08  9:37   ` Richard Palethorpe
  1 sibling, 1 reply; 11+ messages in thread
From: Andrea Cervesato @ 2023-08-31 10:42 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/getegid/getegid02.c | 93 +++++--------------
 1 file changed, 21 insertions(+), 72 deletions(-)

diff --git a/testcases/kernel/syscalls/getegid/getegid02.c b/testcases/kernel/syscalls/getegid/getegid02.c
index 60f09501e..2f64bd869 100644
--- a/testcases/kernel/syscalls/getegid/getegid02.c
+++ b/testcases/kernel/syscalls/getegid/getegid02.c
@@ -1,90 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) International Business Machines  Corp., 2001
- *  Ported by Wayne Boyer
- *
- * 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
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *   William Roske, Dave Fenner
+ * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Testcase to check the basic functionality of getegid().
+/*\
+ * [Description]
  *
- * For functionality test the return value from getegid() is compared to passwd
- * entry.
+ * This test checks if getegid() returns the same effective group given by
+ * passwd entry via getpwuid().
  */
 
 #include <pwd.h>
-#include <errno.h>
-
-#include "test.h"
-#include "compat_16.h"
 
-static void cleanup(void);
-static void setup(void);
+#include "tst_test.h"
+#include "compat_tst_16.h"
 
-TCID_DEFINE(getegid02);
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static void run(void)
 {
-	int lc;
 	uid_t euid;
+	gid_t egid;
 	struct passwd *pwent;
 
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		TEST(GETEGID(cleanup));
+	UID16_CHECK((euid = geteuid()), "geteuid");
 
-		if (TEST_RETURN < 0) {
-			tst_brkm(TBROK, cleanup, "This should never happen");
-		}
+	pwent = getpwuid(euid);
+	if (!pwent)
+		tst_brk(TBROK | TERRNO, "getpwuid() error");
 
-		euid = geteuid();
-		pwent = getpwuid(euid);
+	GID16_CHECK((egid = getegid()), "getegid");
 
-		if (pwent == NULL)
-			tst_brkm(TBROK, cleanup, "geteuid() returned "
-				 "unexpected value %d", euid);
-
-		GID16_CHECK(pwent->pw_gid, getegid, cleanup);
-
-		if (pwent->pw_gid != TEST_RETURN) {
-			tst_resm(TFAIL, "getegid() return value"
-				 " %ld unexpected - expected %d",
-				 TEST_RETURN, pwent->pw_gid);
-		} else {
-			tst_resm(TPASS,
-				 "effective group id %ld "
-				 "is correct", TEST_RETURN);
-		}
-	}
-
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	TST_EXP_EQ_LI(pwent->pw_gid, egid);
 }
 
-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.test_all = run,
+};
-- 
2.35.3


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-08-31 10:42 ` [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API Andrea Cervesato
@ 2023-08-31 13:32   ` Cyril Hrubis
  2023-09-01 11:46     ` Andrea Cervesato via ltp
  2023-09-01 11:57     ` Andrea Cervesato via ltp
  0 siblings, 2 replies; 11+ messages in thread
From: Cyril Hrubis @ 2023-08-31 13:32 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> +	gid_t gid;
> +	gid_t st_gid, st_egid;
>  
> -		if (TEST_RETURN == -1) {
> -			tst_resm(TFAIL | TTERRNO, "getegid failed");
> -			continue;	/* next loop for MTKERNEL */
> -		}
> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);

Hint: You can use assignment suppresion '*' in order to get rid of the
      dummy st_gid variable.

> -		tst_resm(TPASS, "getegid returned %ld", TEST_RETURN);
> -	}
> -
> -	cleanup();
> -	tst_exit();
> -}
> -
> -static void setup(void)
> -{
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -	TEST_PAUSE;
> +	GID16_CHECK((gid = GETEGID()), "getegid");

And this does not work at all.

The GID16_CHECK() is for the case where we have GID that is 32bit and we
want to check if the value fits into 16bit. In this case we get 16bit
value from the sycall, so the check will always be true.

Also the check only returns value, which we ingore here, so this isn't
doing anything at all.

What we need to do instead is to check the gid from /proc/ that is 32bit
if it fits into 16bit (in the case of the 16bit syscall) and skip the
comparsion below.

> +	TST_EXP_EQ_LI(gid, st_egid);

So the code here should really do:

	if (GID16_CHECK(st_egid))
		TST_EXP_EQ_LI(gid, st_egid);
	else
		tst_res(TPASS, "getgid() passed");

Which skips the check on 16bit syscall in the case that the GID
overflows 16bit, however we still have to report at least single TPASS
otherwise the test will be failed by the test library.

>  }
>  
> -static void cleanup(void)
> -{
> -}
> +static struct tst_test test = {
> +	.test_all = run,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-08-31 13:32   ` Cyril Hrubis
@ 2023-09-01 11:46     ` Andrea Cervesato via ltp
  2023-09-01 12:21       ` Cyril Hrubis
  2023-09-01 11:57     ` Andrea Cervesato via ltp
  1 sibling, 1 reply; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2023-09-01 11:46 UTC (permalink / raw)
  To: Cyril Hrubis, Andrea Cervesato; +Cc: ltp

Hi!

On 8/31/23 15:32, Cyril Hrubis wrote:
> Hi!
>> +	gid_t gid;
>> +	gid_t st_gid, st_egid;
>>   
>> -		if (TEST_RETURN == -1) {
>> -			tst_resm(TFAIL | TTERRNO, "getegid failed");
>> -			continue;	/* next loop for MTKERNEL */
>> -		}
>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);
> Hint: You can use assignment suppresion '*' in order to get rid of the
>        dummy st_gid variable.
>
>> -		tst_resm(TPASS, "getegid returned %ld", TEST_RETURN);
>> -	}
>> -
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -	TEST_PAUSE;
>> +	GID16_CHECK((gid = GETEGID()), "getegid");
> And this does not work at all.
>
> The GID16_CHECK() is for the case where we have GID that is 32bit and we
> want to check if the value fits into 16bit. In this case we get 16bit
> value from the sycall, so the check will always be true.
>
> Also the check only returns value, which we ingore here, so this isn't
> doing anything at all.
>
> What we need to do instead is to check the gid from /proc/ that is 32bit
> if it fits into 16bit (in the case of the 16bit syscall) and skip the
> comparsion below.
>
>> +	TST_EXP_EQ_LI(gid, st_egid);
> So the code here should really do:
>
> 	if (GID16_CHECK(st_egid))
> 		TST_EXP_EQ_LI(gid, st_egid);
> 	else
> 		tst_res(TPASS, "getgid() passed");

Mmmh, this would make sense if GID16_CHECK only checked for 16bit 
compatibility, because at the moment it's way different:


#define GID16_CHECK(gid, sys_name, cleanup) \
if (!GID_SIZE_CHECK(gid)) { \
     tst_brkm(TBROK, cleanup, \
         "gid %d of %s is too large for testing 16-bit version " \
         "of %s()", gid, #gid, #sys_name); \
}

Am I missing something?

> Which skips the check on 16bit syscall in the case that the GID
> overflows 16bit, however we still have to report at least single TPASS
> otherwise the test will be failed by the test library.
>
>>   }
>>   
>> -static void cleanup(void)
>> -{
>> -}
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +};
>> -- 
>> 2.35.3
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Andrea


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-08-31 13:32   ` Cyril Hrubis
  2023-09-01 11:46     ` Andrea Cervesato via ltp
@ 2023-09-01 11:57     ` Andrea Cervesato via ltp
  2023-09-01 12:22       ` Cyril Hrubis
  1 sibling, 1 reply; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2023-09-01 11:57 UTC (permalink / raw)
  To: Cyril Hrubis, Andrea Cervesato; +Cc: ltp

Hi,

the last thing.

On 8/31/23 15:32, Cyril Hrubis wrote:
> Hi!
>> +	gid_t gid;
>> +	gid_t st_gid, st_egid;
>>   
>> -		if (TEST_RETURN == -1) {
>> -			tst_resm(TFAIL | TTERRNO, "getegid failed");
>> -			continue;	/* next loop for MTKERNEL */
>> -		}
>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);
> Hint: You can use assignment suppresion '*' in order to get rid of the
>        dummy st_gid variable.
Also, unfortunately this is not working.
>
>> -		tst_resm(TPASS, "getegid returned %ld", TEST_RETURN);
>> -	}
>> -
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -	TEST_PAUSE;
>> +	GID16_CHECK((gid = GETEGID()), "getegid");
> And this does not work at all.
>
> The GID16_CHECK() is for the case where we have GID that is 32bit and we
> want to check if the value fits into 16bit. In this case we get 16bit
> value from the sycall, so the check will always be true.
>
> Also the check only returns value, which we ingore here, so this isn't
> doing anything at all.
>
> What we need to do instead is to check the gid from /proc/ that is 32bit
> if it fits into 16bit (in the case of the 16bit syscall) and skip the
> comparsion below.
>
>> +	TST_EXP_EQ_LI(gid, st_egid);
> So the code here should really do:
>
> 	if (GID16_CHECK(st_egid))
> 		TST_EXP_EQ_LI(gid, st_egid);
> 	else
> 		tst_res(TPASS, "getgid() passed");
>
> Which skips the check on 16bit syscall in the case that the GID
> overflows 16bit, however we still have to report at least single TPASS
> otherwise the test will be failed by the test library.
>
>>   }
>>   
>> -static void cleanup(void)
>> -{
>> -}
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +};
>> -- 
>> 2.35.3
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Andrea


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-09-01 11:46     ` Andrea Cervesato via ltp
@ 2023-09-01 12:21       ` Cyril Hrubis
  0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2023-09-01 12:21 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> >> +	TST_EXP_EQ_LI(gid, st_egid);
> > So the code here should really do:
> >
> > 	if (GID16_CHECK(st_egid))
> > 		TST_EXP_EQ_LI(gid, st_egid);
> > 	else
> > 		tst_res(TPASS, "getgid() passed");
> 
> Mmmh, this would make sense if GID16_CHECK only checked for 16bit 
> compatibility, because at the moment it's way different:
> 
> 
> #define GID16_CHECK(gid, sys_name, cleanup) \
> if (!GID_SIZE_CHECK(gid)) { \
>      tst_brkm(TBROK, cleanup, \
>          "gid %d of %s is too large for testing 16-bit version " \
>          "of %s()", gid, #gid, #sys_name); \
> }
> 
> Am I missing something?

It seems that I confused GID16_CHECK with GID_SIZE_CHECK(), so it should
have been:

	if (GID_SIZE_CHECK(st_egid))

> > Which skips the check on 16bit syscall in the case that the GID
> > overflows 16bit, however we still have to report at least single TPASS
> > otherwise the test will be failed by the test library.
> >
> >>   }
> >>   
> >> -static void cleanup(void)
> >> -{
> >> -}
> >> +static struct tst_test test = {
> >> +	.test_all = run,
> >> +};
> >> -- 
> >> 2.35.3
> >>
> >>
> >> -- 
> >> Mailing list info: https://lists.linux.it/listinfo/ltp
> 
> Andrea
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-09-01 11:57     ` Andrea Cervesato via ltp
@ 2023-09-01 12:22       ` Cyril Hrubis
  2023-09-01 12:47         ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2023-09-01 12:22 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> >> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);
> > Hint: You can use assignment suppresion '*' in order to get rid of the
> >        dummy st_gid variable.
> Also, unfortunately this is not working.

SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %*d %d", &st_egid); works for me.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API
  2023-09-01 12:22       ` Cyril Hrubis
@ 2023-09-01 12:47         ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2023-09-01 12:47 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi,

On 9/1/23 14:22, Cyril Hrubis wrote:
> Hi!
>>>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %d %d", &st_gid, &st_egid);
>>> Hint: You can use assignment suppresion '*' in order to get rid of the
>>>         dummy st_gid variable.
>> Also, unfortunately this is not working.
> SAFE_FILE_LINES_SCANF("/proc/self/status", "Gid: %*d %d", &st_egid); works for me.
>
Ok, that perfectly works now.

Waiting for getegid02 review, then I will modify code and send it with 
the patch set.

Thanks,
Andrea


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 2/2] Refactor getegid02 using new LTP API
  2023-08-31 10:42 ` [LTP] [PATCH v1 2/2] Refactor getegid02 " Andrea Cervesato
@ 2023-09-08  9:37   ` Richard Palethorpe
  2023-09-08 10:25     ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 11+ messages in thread
From: Richard Palethorpe @ 2023-09-08  9:37 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hello,

Looks good, I guess you will make changes to the first one and resend?

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

Andrea Cervesato <andrea.cervesato@suse.de> writes:

> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  testcases/kernel/syscalls/getegid/getegid02.c | 93 +++++--------------
>  1 file changed, 21 insertions(+), 72 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/getegid/getegid02.c b/testcases/kernel/syscalls/getegid/getegid02.c
> index 60f09501e..2f64bd869 100644
> --- a/testcases/kernel/syscalls/getegid/getegid02.c
> +++ b/testcases/kernel/syscalls/getegid/getegid02.c
> @@ -1,90 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * Copyright (c) International Business Machines  Corp., 2001
> - *  Ported by Wayne Boyer
> - *
> - * 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
> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
> + *   William Roske, Dave Fenner
> + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>   */
>  
> -/*
> - * Testcase to check the basic functionality of getegid().
> +/*\
> + * [Description]
>   *
> - * For functionality test the return value from getegid() is compared to passwd
> - * entry.
> + * This test checks if getegid() returns the same effective group given by
> + * passwd entry via getpwuid().
>   */
>  
>  #include <pwd.h>
> -#include <errno.h>
> -
> -#include "test.h"
> -#include "compat_16.h"
>  
> -static void cleanup(void);
> -static void setup(void);
> +#include "tst_test.h"
> +#include "compat_tst_16.h"
>  
> -TCID_DEFINE(getegid02);
> -int TST_TOTAL = 1;
> -
> -int main(int ac, char **av)
> +static void run(void)
>  {
> -	int lc;
>  	uid_t euid;
> +	gid_t egid;
>  	struct passwd *pwent;
>  
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		TEST(GETEGID(cleanup));
> +	UID16_CHECK((euid = geteuid()), "geteuid");
>  
> -		if (TEST_RETURN < 0) {
> -			tst_brkm(TBROK, cleanup, "This should never happen");
> -		}
> +	pwent = getpwuid(euid);
> +	if (!pwent)
> +		tst_brk(TBROK | TERRNO, "getpwuid() error");
>  
> -		euid = geteuid();
> -		pwent = getpwuid(euid);
> +	GID16_CHECK((egid = getegid()), "getegid");
>  
> -		if (pwent == NULL)
> -			tst_brkm(TBROK, cleanup, "geteuid() returned "
> -				 "unexpected value %d", euid);
> -
> -		GID16_CHECK(pwent->pw_gid, getegid, cleanup);
> -
> -		if (pwent->pw_gid != TEST_RETURN) {
> -			tst_resm(TFAIL, "getegid() return value"
> -				 " %ld unexpected - expected %d",
> -				 TEST_RETURN, pwent->pw_gid);
> -		} else {
> -			tst_resm(TPASS,
> -				 "effective group id %ld "
> -				 "is correct", TEST_RETURN);
> -		}
> -	}
> -
> -	cleanup();
> -	tst_exit();
> -}
> -
> -static void setup(void)
> -{
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -	TEST_PAUSE;
> +	TST_EXP_EQ_LI(pwent->pw_gid, egid);
>  }
>  
> -static void cleanup(void)
> -{
> -}
> +static struct tst_test test = {
> +	.test_all = run,
> +};
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH v1 2/2] Refactor getegid02 using new LTP API
  2023-09-08  9:37   ` Richard Palethorpe
@ 2023-09-08 10:25     ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2023-09-08 10:25 UTC (permalink / raw)
  To: rpalethorpe, Andrea Cervesato; +Cc: ltp

Hi!

I think you can push both getegid01 and getegid02 together, as Cyril 
mentioned in the getegid01 review.
Sorry, it was indeed better to create a patch set for this.

Thanks!
Andrea Cervesato

On 9/8/23 11:37, Richard Palethorpe wrote:
> Hello,
>
> Looks good, I guess you will make changes to the first one and resend?
>
> Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
>
> Andrea Cervesato <andrea.cervesato@suse.de> writes:
>
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   testcases/kernel/syscalls/getegid/getegid02.c | 93 +++++--------------
>>   1 file changed, 21 insertions(+), 72 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/getegid/getegid02.c b/testcases/kernel/syscalls/getegid/getegid02.c
>> index 60f09501e..2f64bd869 100644
>> --- a/testcases/kernel/syscalls/getegid/getegid02.c
>> +++ b/testcases/kernel/syscalls/getegid/getegid02.c
>> @@ -1,90 +1,39 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>> - * Copyright (c) International Business Machines  Corp., 2001
>> - *  Ported by Wayne Boyer
>> - *
>> - * 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
>> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>> + *   William Roske, Dave Fenner
>> + * Copyright (C) 2023 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>>    */
>>   
>> -/*
>> - * Testcase to check the basic functionality of getegid().
>> +/*\
>> + * [Description]
>>    *
>> - * For functionality test the return value from getegid() is compared to passwd
>> - * entry.
>> + * This test checks if getegid() returns the same effective group given by
>> + * passwd entry via getpwuid().
>>    */
>>   
>>   #include <pwd.h>
>> -#include <errno.h>
>> -
>> -#include "test.h"
>> -#include "compat_16.h"
>>   
>> -static void cleanup(void);
>> -static void setup(void);
>> +#include "tst_test.h"
>> +#include "compat_tst_16.h"
>>   
>> -TCID_DEFINE(getegid02);
>> -int TST_TOTAL = 1;
>> -
>> -int main(int ac, char **av)
>> +static void run(void)
>>   {
>> -	int lc;
>>   	uid_t euid;
>> +	gid_t egid;
>>   	struct passwd *pwent;
>>   
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		TEST(GETEGID(cleanup));
>> +	UID16_CHECK((euid = geteuid()), "geteuid");
>>   
>> -		if (TEST_RETURN < 0) {
>> -			tst_brkm(TBROK, cleanup, "This should never happen");
>> -		}
>> +	pwent = getpwuid(euid);
>> +	if (!pwent)
>> +		tst_brk(TBROK | TERRNO, "getpwuid() error");
>>   
>> -		euid = geteuid();
>> -		pwent = getpwuid(euid);
>> +	GID16_CHECK((egid = getegid()), "getegid");
>>   
>> -		if (pwent == NULL)
>> -			tst_brkm(TBROK, cleanup, "geteuid() returned "
>> -				 "unexpected value %d", euid);
>> -
>> -		GID16_CHECK(pwent->pw_gid, getegid, cleanup);
>> -
>> -		if (pwent->pw_gid != TEST_RETURN) {
>> -			tst_resm(TFAIL, "getegid() return value"
>> -				 " %ld unexpected - expected %d",
>> -				 TEST_RETURN, pwent->pw_gid);
>> -		} else {
>> -			tst_resm(TPASS,
>> -				 "effective group id %ld "
>> -				 "is correct", TEST_RETURN);
>> -		}
>> -	}
>> -
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -	TEST_PAUSE;
>> +	TST_EXP_EQ_LI(pwent->pw_gid, egid);
>>   }
>>   
>> -static void cleanup(void)
>> -{
>> -}
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +};
>> -- 
>> 2.35.3
>


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-09-08 10:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-31 10:42 [LTP] [PATCH v1 0/2] Refactor getegid testing suite Andrea Cervesato
2023-08-31 10:42 ` [LTP] [PATCH v1 1/2] Refactor getegid01 using new LTP API Andrea Cervesato
2023-08-31 13:32   ` Cyril Hrubis
2023-09-01 11:46     ` Andrea Cervesato via ltp
2023-09-01 12:21       ` Cyril Hrubis
2023-09-01 11:57     ` Andrea Cervesato via ltp
2023-09-01 12:22       ` Cyril Hrubis
2023-09-01 12:47         ` Andrea Cervesato via ltp
2023-08-31 10:42 ` [LTP] [PATCH v1 2/2] Refactor getegid02 " Andrea Cervesato
2023-09-08  9:37   ` Richard Palethorpe
2023-09-08 10:25     ` Andrea Cervesato via ltp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox