| Message ID | 20210303023235.431238-1-yangx.jy@cn.fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | syscalls: Use anonymous .resource_files for docparse | expand |
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
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 >
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
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
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.
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
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.
> 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
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
Hi! > "execveat02": { > - "resource_files": "resource_files", > + "resource_files": [ > + "TEST_APP" > + ], > > => improvement => I'd merge this patch. Agreed, can we merge this patch? Or is there a reason not to? > 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" I guess that this point has been discussed in the "metaparse: Replace macro also in arrays" where we agreed that we will need a list of macros that should not be expandened.
Hi all, > Hi! > > "execveat02": { > > - "resource_files": "resource_files", > > + "resource_files": [ > > + "TEST_APP" > > + ], > > => improvement => I'd merge this patch. > Agreed, can we merge this patch? Or is there a reason not to? I just merge it. > > 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" > I guess that this point has been discussed in the > "metaparse: Replace macro also in arrays" > where we agreed that we will need a list of macros that should not be > expandened. Yes, I'll still plan to finish it, but first more urgent work must be done :). Kind regards, Petr
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,
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(-)