diff mbox series

syscalls: Use anonymous .resource_files for docparse

Message ID 20210303023235.431238-1-yangx.jy@cn.fujitsu.com
State New
Headers show
Series syscalls: Use anonymous .resource_files for docparse | expand

Commit Message

Xiao Yang March 3, 2021, 2:32 a.m. UTC
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/creat/creat07.c       | 10 ++++------
 testcases/kernel/syscalls/execve/execve02.c     | 10 ++++------
 testcases/kernel/syscalls/execve/execve04.c     | 10 ++++------
 testcases/kernel/syscalls/execve/execve05.c     | 10 ++++------
 testcases/kernel/syscalls/execveat/execveat01.c | 10 ++++------
 testcases/kernel/syscalls/execveat/execveat02.c | 10 ++++------
 testcases/kernel/syscalls/execveat/execveat03.c | 10 ++++------
 testcases/kernel/syscalls/fanotify/fanotify10.c | 10 ++++------
 testcases/kernel/syscalls/fanotify/fanotify12.c | 10 ++++------
 testcases/kernel/syscalls/pipe2/pipe2_02.c      | 10 ++++------
 testcases/kernel/syscalls/prctl/prctl06.c       | 10 ++++------
 11 files changed, 44 insertions(+), 66 deletions(-)

Comments

Petr Vorel March 10, 2021, 10:32 a.m. UTC | #1
Hi,

> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>

> -static const char *const resource_files[] = {
> -	TEST_APP,
> -	NULL,
> -};
> -
>  static struct tst_test test = {
>  	.test_all = verify_creat,
>  	.needs_checkpoints = 1,
>  	.forks_child = 1,
> -	.resource_files = resource_files,
> +	.resource_files = (const char *const []) {
> +		TEST_APP,
Don't we want to drop TEST_APP definition and use file directly?
Having TEST_APP does not say much.
Not sure how far we should go with moving everything into inline anonymous
definitions (it'd be nice if docparse was able to just expand macros, but that
would be way too slow).

Kind regards,
Petr
Xiao Yang March 10, 2021, 2:54 p.m. UTC | #2
On 3/10/21 6:32 PM, Petr Vorel wrote:
> Hi,
>
>> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
>> -static const char *const resource_files[] = {
>> -	TEST_APP,
>> -	NULL,
>> -};
>> -
>>   static struct tst_test test = {
>>   	.test_all = verify_creat,
>>   	.needs_checkpoints = 1,
>>   	.forks_child = 1,
>> -	.resource_files = resource_files,
>> +	.resource_files = (const char *const []) {
>> +		TEST_APP,
> Don't we want to drop TEST_APP definition and use file directly?
> Having TEST_APP does not say much.

Hi Petr,


I can use file name directly in v2 patch.

Do you agree to use anonymous .resource_files for now? or is it better 
to keep it?


> Not sure how far we should go with moving everything into inline anonymous
> definitions (it'd be nice if docparse was able to just expand macros, but that
> would be way too slow).

I agree that expanding macros or structs is the nicer way but

we need to do some investigation about it.


Best Regards,

Xiao Yang

>
> Kind regards,
> Petr
>
Petr Vorel March 10, 2021, 5:21 p.m. UTC | #3
Hi Yang,

> On 3/10/21 6:32 PM, Petr Vorel wrote:
> > Hi,

> > > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > > -static const char *const resource_files[] = {
> > > -	TEST_APP,
> > > -	NULL,
> > > -};
> > > -
> > >   static struct tst_test test = {
> > >   	.test_all = verify_creat,
> > >   	.needs_checkpoints = 1,
> > >   	.forks_child = 1,
> > > -	.resource_files = resource_files,
> > > +	.resource_files = (const char *const []) {
> > > +		TEST_APP,
> > Don't we want to drop TEST_APP definition and use file directly?
> > Having TEST_APP does not say much.

> Hi Petr,


> I can use file name directly in v2 patch.
OK. I'd like to know the others opinion (precedent).

> Do you agree to use anonymous .resource_files for now? or is it better to
> keep it?
I guess yes, we've already started to use it.

> > Not sure how far we should go with moving everything into inline anonymous
> > definitions (it'd be nice if docparse was able to just expand macros, but that
> > would be way too slow).

> I agree that expanding macros or structs is the nicer way but

> we need to do some investigation about it.
gcc -E foo.c would do expansion for us. But not sure if it's worth of runtime.
Because problem of missing definitions will be on other places and we don't want
to get rid of definitions. e.g. I planned to add some tag definitions (for
"linux-git", ...) as Martin Doucha suggested, but this would not work until
we expand macros.

Kind regards,
Petr

> Best Regards,

> Xiao Yang
Petr Vorel March 11, 2021, 10:33 a.m. UTC | #4
Hi,

...
> > Hi Petr,

> > I can use file name directly in v2 patch.
> OK. I'd like to know the others opinion (precedent).

> > Do you agree to use anonymous .resource_files for now? or is it better to
> > keep it?
> I guess yes, we've already started to use it.

> > > Not sure how far we should go with moving everything into inline anonymous
> > > definitions (it'd be nice if docparse was able to just expand macros, but that
> > > would be way too slow).

> > I agree that expanding macros or structs is the nicer way but

> > we need to do some investigation about it.
> gcc -E foo.c would do expansion for us. But not sure if it's worth of runtime.
> Because problem of missing definitions will be on other places and we don't want
> to get rid of definitions. e.g. I planned to add some tag definitions (for
> "linux-git", ...) as Martin Doucha suggested, but this would not work until
> we expand macros.

I was looking into the output of gcc -E but it brings other problems.
Wouldn't be better instead of patching like this to just replace docparse.c with
library support to test itself print it's description in json format
(e.g. --print-json opt)? I was thinking to use the same for shell tests docparse
(which aren't covered at all yet).

The downside would be that generation would be much slower and require native
build.

> Kind regards,
> Petr

> > Best Regards,

> > Xiao Yang
Cyril Hrubis March 11, 2021, 10:42 a.m. UTC | #5
Hi!
> > > we need to do some investigation about it.
> > gcc -E foo.c would do expansion for us. But not sure if it's worth of runtime.
> > Because problem of missing definitions will be on other places and we don't want
> > to get rid of definitions. e.g. I planned to add some tag definitions (for
> > "linux-git", ...) as Martin Doucha suggested, but this would not work until
> > we expand macros.
> 
> I was looking into the output of gcc -E but it brings other problems.
> Wouldn't be better instead of patching like this to just replace docparse.c with
> library support to test itself print it's description in json format
> (e.g. --print-json opt)? I was thinking to use the same for shell tests docparse
> (which aren't covered at all yet).

I've been there and tried that that was v1 of the proposal, it did not work.

We can add macro expansion to the docparse instead, it shouldn't be that
hard.
Petr Vorel March 12, 2021, 11:05 a.m. UTC | #6
Hi Cyril,

> Hi!
> > > > we need to do some investigation about it.
> > > gcc -E foo.c would do expansion for us. But not sure if it's worth of runtime.
> > > Because problem of missing definitions will be on other places and we don't want
> > > to get rid of definitions. e.g. I planned to add some tag definitions (for
> > > "linux-git", ...) as Martin Doucha suggested, but this would not work until
> > > we expand macros.

> > I was looking into the output of gcc -E but it brings other problems.
> > Wouldn't be better instead of patching like this to just replace docparse.c with
> > library support to test itself print it's description in json format
> > (e.g. --print-json opt)? I was thinking to use the same for shell tests docparse
> > (which aren't covered at all yet).

> I've been there and tried that that was v1 of the proposal, it did not work.
OK, thanks for info.

> We can add macro expansion to the docparse instead, it shouldn't be that
> hard.
Not sure if I understand what you mean. Using gcc -E or something else?

Kind regards,
Petr
Cyril Hrubis March 12, 2021, 1:12 p.m. UTC | #7
Hi!
> > We can add macro expansion to the docparse instead, it shouldn't be that
> > hard.
> Not sure if I understand what you mean. Using gcc -E or something else?

Not really, we can just add a has table and put all macros in there
while we traverse the code. I can prototype code for that later on.
Petr Vorel March 12, 2021, 1:24 p.m. UTC | #8
> Hi!
> > > We can add macro expansion to the docparse instead, it shouldn't be that
> > > hard.
> > Not sure if I understand what you mean. Using gcc -E or something else?

> Not really, we can just add a has table and put all macros in there
> while we traverse the code. I can prototype code for that later on.

Right, sounds good. Thanks for info.

Kind regards,
Petr
Petr Vorel Feb. 25, 2022, 12:37 p.m. UTC | #9
Hi all,

Changes in  ltp.json after Cyrils enhancements to parser:

  "execveat02": {
-   "resource_files": "resource_files",
+   "resource_files": [
+     "TEST_APP"
+    ],

=> improvement => I'd merge this patch.


NOTE: It'd be nice if even TEST_APP was changed to execveat_child,
not sure what needs to be one for it. NOTE the definitions are mostly in C
source:

#define TEST_APP "execveat_child"

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/creat/creat07.c b/testcases/kernel/syscalls/creat/creat07.c
index 1e9779476..7bd32ab4d 100644
--- a/testcases/kernel/syscalls/creat/creat07.c
+++ b/testcases/kernel/syscalls/creat/creat07.c
@@ -47,14 +47,12 @@  static void verify_creat(void)
 	SAFE_WAITPID(pid, NULL, 0);
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static struct tst_test test = {
 	.test_all = verify_creat,
 	.needs_checkpoints = 1,
 	.forks_child = 1,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	}
 };
diff --git a/testcases/kernel/syscalls/execve/execve02.c b/testcases/kernel/syscalls/execve/execve02.c
index d9fb5b919..4e6be826b 100644
--- a/testcases/kernel/syscalls/execve/execve02.c
+++ b/testcases/kernel/syscalls/execve/execve02.c
@@ -74,16 +74,14 @@  static void setup(void)
 	nobody_uid = pwd->pw_uid;
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.setup = setup,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.test_all = verify_execve,
 };
diff --git a/testcases/kernel/syscalls/execve/execve04.c b/testcases/kernel/syscalls/execve/execve04.c
index c7b8c1614..18e883ab3 100644
--- a/testcases/kernel/syscalls/execve/execve04.c
+++ b/testcases/kernel/syscalls/execve/execve04.c
@@ -63,15 +63,13 @@  static void do_child(void)
 	exit(0);
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static struct tst_test test = {
 	.test_all = verify_execve,
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.needs_checkpoints = 1,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	}
 };
diff --git a/testcases/kernel/syscalls/execve/execve05.c b/testcases/kernel/syscalls/execve/execve05.c
index 4c9789cc5..a26eba79a 100644
--- a/testcases/kernel/syscalls/execve/execve05.c
+++ b/testcases/kernel/syscalls/execve/execve05.c
@@ -44,11 +44,6 @@  static int nchild = 8;
 
 static char *opt_nchild;
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static void do_child(void)
 {
 	char *argv[3] = {TEST_APP, "canary", NULL};
@@ -86,6 +81,9 @@  static struct tst_test test = {
 	.forks_child = 1,
 	.child_needs_reinit = 1,
 	.needs_checkpoints = 1,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.setup = setup,
 };
diff --git a/testcases/kernel/syscalls/execveat/execveat01.c b/testcases/kernel/syscalls/execveat/execveat01.c
index 16d27acf6..55891b74c 100644
--- a/testcases/kernel/syscalls/execveat/execveat01.c
+++ b/testcases/kernel/syscalls/execveat/execveat01.c
@@ -84,13 +84,11 @@  static void cleanup(void)
 		SAFE_CLOSE(fd4);
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static struct tst_test test = {
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = verify_execveat,
 	.child_needs_reinit = 1,
diff --git a/testcases/kernel/syscalls/execveat/execveat02.c b/testcases/kernel/syscalls/execveat/execveat02.c
index 9b08efb78..c057b8eaf 100644
--- a/testcases/kernel/syscalls/execveat/execveat02.c
+++ b/testcases/kernel/syscalls/execveat/execveat02.c
@@ -85,11 +85,6 @@  static void setup(void)
 	fd = SAFE_OPEN(TEST_REL_APP, O_PATH);
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static void cleanup(void)
 {
 	if (fd > 0)
@@ -97,7 +92,10 @@  static void cleanup(void)
 }
 
 static struct tst_test test = {
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.tcnt = ARRAY_SIZE(tcases),
 	.test = verify_execveat,
 	.child_needs_reinit = 1,
diff --git a/testcases/kernel/syscalls/execveat/execveat03.c b/testcases/kernel/syscalls/execveat/execveat03.c
index 78b26ab56..97df8f33e 100644
--- a/testcases/kernel/syscalls/execveat/execveat03.c
+++ b/testcases/kernel/syscalls/execveat/execveat03.c
@@ -68,11 +68,6 @@  static void setup(void)
 	check_execveat();
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL,
-};
-
 static struct tst_test test = {
 	.needs_root = 1,
 	.mount_device = 1,
@@ -82,7 +77,10 @@  static struct tst_test test = {
 	.child_needs_reinit = 1,
 	.setup = setup,
 	.test_all = verify_execveat,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "8db6c34f1dbc"},
 		{"linux-git", "355139a8dba4"},
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index eeba87568..b2eb909a7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -534,11 +534,6 @@  static void cleanup(void)
 		tst_brk(TBROK | TERRNO, "bind umount failed");
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL
-};
-
 static struct tst_test test = {
 	.test = test_fanotify,
 	.tcnt = ARRAY_SIZE(tcases),
@@ -548,7 +543,10 @@  static struct tst_test test = {
 	.mntpoint = MOUNT_PATH,
 	.needs_root = 1,
 	.forks_child = 1,
-	.resource_files = resource_files,
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "9bdda4e9cf2d"},
 		{"linux-git", "2f02fd3fa13e"},
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index 17086ef71..7070b9e4f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -232,11 +232,6 @@  static void do_cleanup(void)
 		SAFE_CLOSE(fd_notify);
 }
 
-static const char *const resource_files[] = {
-	TEST_APP,
-	NULL
-};
-
 static struct tst_test test = {
 	.setup = do_setup,
 	.test = do_test,
@@ -244,7 +239,10 @@  static struct tst_test test = {
 	.cleanup = do_cleanup,
 	.forks_child = 1,
 	.needs_root = 1,
-	.resource_files = resource_files
+	.resource_files = (const char *const []) {
+		TEST_APP,
+		NULL
+	}
 };
 #else
 	TST_TEST_TCONF("System does not contain required fanotify support");
diff --git a/testcases/kernel/syscalls/pipe2/pipe2_02.c b/testcases/kernel/syscalls/pipe2/pipe2_02.c
index 9ba69667b..ee317668b 100644
--- a/testcases/kernel/syscalls/pipe2/pipe2_02.c
+++ b/testcases/kernel/syscalls/pipe2/pipe2_02.c
@@ -54,13 +54,11 @@  static void verify_pipe2(void)
 	cleanup();
 }
 
-static const char *const resfile[] = {
-	TESTBIN,
-	NULL,
-};
-
 static struct tst_test test = {
-	.resource_files = resfile,
+	.resource_files = (const char *const []) {
+		TESTBIN,
+		NULL
+	},
 	.cleanup = cleanup,
 	.forks_child = 1,
 	.needs_root = 1,
diff --git a/testcases/kernel/syscalls/prctl/prctl06.c b/testcases/kernel/syscalls/prctl/prctl06.c
index 21d336c07..2395f1adc 100644
--- a/testcases/kernel/syscalls/prctl/prctl06.c
+++ b/testcases/kernel/syscalls/prctl/prctl06.c
@@ -107,13 +107,11 @@  static void setup(void)
 		"current environment doesn't permit PR_GET/SET_NO_NEW_PRIVS");
 }
 
-static const char *const resfile[] = {
-	TESTBIN,
-	NULL,
-};
-
 static struct tst_test test = {
-	.resource_files = resfile,
+	.resource_files = (const char *const []) {
+		TESTBIN,
+		NULL
+	},
 	.setup = setup,
 	.test_all = verify_prctl,
 	.forks_child = 1,