2022-07-29 15:36:43

by Petr Vorel

[permalink] [raw]
Subject: [RFC PATCH 1/1] metaparse: Replace macro also in arrays

This helps to replace macros like:

#define TEST_APP "userns06_capcheck"

static const char *const resource_files[] = {
TEST_APP,
NULL,
};

$ ./metaparse -Iinclude -Itestcases/kernel/syscalls/utils/ ../testcases/kernel/containers/userns/userns06.c
Before:
"resource_files": [
"TEST_APP"
],
...

After:
"resource_files": [
"userns06_capcheck"
],
...

Signed-off-by: Petr Vorel <[email protected]>
---
Hi all,

This is a reaction on patch
https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/
First: I was wrong, inlining arrays does any change in the docparse output.
BTW I'd be still for inlining for better readability.

I'm not sure if this is not good idea, maybe some of the constants should be
kept unparsed, e.g.:

Orig:
"caps": [
"TST_CAP",
"(",
"TST_CAP_DROP",
"CAP_SYS_RESOURCE",

Becomes:
"caps": [
"TST_CAP",
"(",
"TST_CAP_DROP",
"24",

CAP_SYS_RESOURCE is replaced because it's a string, but IMHO it'd be better to keep it.
TST_CAP{_DROP,} aren't replaced because they aren't a plain strings.
Maybe replace only non-numerc values?

Kind regards,
Petr

metadata/metaparse.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 2384c73c8..0cc288b2d 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Copyright (c) 2019-2021 Cyril Hrubis <[email protected]>
- * Copyright (c) 2020 Petr Vorel <[email protected]>
+ * Copyright (c) 2020-2022 Petr Vorel <[email protected]>
*/

#define _GNU_SOURCE
@@ -286,9 +286,28 @@ static void close_include(FILE *inc)
fclose(inc);
}

+static void try_apply_macro(char **res)
+{
+ ENTRY macro = {
+ .key = *res,
+ };
+
+ ENTRY *ret;
+
+ ret = hsearch(macro, FIND);
+
+ if (!ret)
+ return;
+
+ if (verbose)
+ fprintf(stderr, "APPLYING MACRO %s=%s\n", ret->key, (char*)ret->data);
+
+ *res = ret->data;
+}
+
static int parse_array(FILE *f, struct data_node *node)
{
- const char *token;
+ char *token;

for (;;) {
if (!(token = next_token(f, NULL)))
@@ -315,6 +334,7 @@ static int parse_array(FILE *f, struct data_node *node)
if (!strcmp(token, "NULL"))
continue;

+ try_apply_macro(&token);
struct data_node *str = data_node_string(token);

data_node_array_add(node, str);
@@ -323,25 +343,6 @@ static int parse_array(FILE *f, struct data_node *node)
return 0;
}

-static void try_apply_macro(char **res)
-{
- ENTRY macro = {
- .key = *res,
- };
-
- ENTRY *ret;
-
- ret = hsearch(macro, FIND);
-
- if (!ret)
- return;
-
- if (verbose)
- fprintf(stderr, "APPLYING MACRO %s=%s\n", ret->key, (char*)ret->data);
-
- *res = ret->data;
-}
-
static int parse_get_array_len(FILE *f)
{
const char *token;
--
2.37.1


2022-08-01 13:00:48

by Cyril Hrubis

[permalink] [raw]
Subject: Re: [RFC PATCH 1/1] metaparse: Replace macro also in arrays

Hi!
> This helps to replace macros like:
>
> #define TEST_APP "userns06_capcheck"
>
> static const char *const resource_files[] = {
> TEST_APP,
> NULL,
> };
>
> $ ./metaparse -Iinclude -Itestcases/kernel/syscalls/utils/ ../testcases/kernel/containers/userns/userns06.c
> Before:
> "resource_files": [
> "TEST_APP"
> ],
> ...
>
> After:
> "resource_files": [
> "userns06_capcheck"
> ],
> ...
>
> Signed-off-by: Petr Vorel <[email protected]>
> ---
> Hi all,
>
> This is a reaction on patch
> https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/
> First: I was wrong, inlining arrays does any change in the docparse output.
> BTW I'd be still for inlining for better readability.
>
> I'm not sure if this is not good idea, maybe some of the constants should be
> kept unparsed, e.g.:
>
> Orig:
> "caps": [
> "TST_CAP",
> "(",
> "TST_CAP_DROP",
> "CAP_SYS_RESOURCE",
>
> Becomes:
> "caps": [
> "TST_CAP",
> "(",
> "TST_CAP_DROP",
> "24",
>
> CAP_SYS_RESOURCE is replaced because it's a string, but IMHO it'd be better to keep it.
> TST_CAP{_DROP,} aren't replaced because they aren't a plain strings.
> Maybe replace only non-numerc values?

That really depends on the context, we do have many cases where we have
a macro that expands to numeric that should be expanded, runtime would
be one of the prime examples of that.

I guess that the only solution would be an explicit list of macro
prefixes that should not be expanded, e.g. do not expand if macro starts
with "CAP_".

--
Cyril Hrubis
[email protected]

2022-08-01 14:37:34

by Petr Vorel

[permalink] [raw]
Subject: Re: [RFC PATCH 1/1] metaparse: Replace macro also in arrays

> Hi!
> > This helps to replace macros like:

> > #define TEST_APP "userns06_capcheck"

> > static const char *const resource_files[] = {
> > TEST_APP,
> > NULL,
> > };

> > $ ./metaparse -Iinclude -Itestcases/kernel/syscalls/utils/ ../testcases/kernel/containers/userns/userns06.c
> > Before:
> > "resource_files": [
> > "TEST_APP"
> > ],
> > ...

> > After:
> > "resource_files": [
> > "userns06_capcheck"
> > ],
> > ...

> > Signed-off-by: Petr Vorel <[email protected]>
> > ---
> > Hi all,

> > This is a reaction on patch
> > https://patchwork.ozlabs.org/project/ltp/patch/[email protected]/
> > First: I was wrong, inlining arrays does any change in the docparse output.
> > BTW I'd be still for inlining for better readability.

> > I'm not sure if this is not good idea, maybe some of the constants should be
> > kept unparsed, e.g.:

> > Orig:
> > "caps": [
> > "TST_CAP",
> > "(",
> > "TST_CAP_DROP",
> > "CAP_SYS_RESOURCE",

> > Becomes:
> > "caps": [
> > "TST_CAP",
> > "(",
> > "TST_CAP_DROP",
> > "24",

> > CAP_SYS_RESOURCE is replaced because it's a string, but IMHO it'd be better to keep it.
> > TST_CAP{_DROP,} aren't replaced because they aren't a plain strings.
> > Maybe replace only non-numerc values?

> That really depends on the context, we do have many cases where we have
> a macro that expands to numeric that should be expanded, runtime would
> be one of the prime examples of that.
Yep.

> I guess that the only solution would be an explicit list of macro
> prefixes that should not be expanded, e.g. do not expand if macro starts
> with "CAP_".

Good idea, thanks! I was thinking about rewrite parsing to sparse, but you would
not like it as being slow. This is indeed a better solution.

Kind regards,
Petr