Re: [LTP] [RFC PATCH 1/1] metaparse: mntpoint implies needs_tmpdir

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> 
> I see lib/tst_test.c
> static int needs_tmpdir(void)
> {
> 	return tst_test->needs_tmpdir ||
> 	       tst_test->needs_device ||
> 	       tst_test->mntpoint ||
> 	       tst_test->resource_files ||
> 	       tst_test->needs_checkpoints;
> }
> 
> But
> 1) It does not detect it.
> 2) -Warray-bounds is introduced.
> 
> What am I missing?
> I originally come to this via
> https://patchwork.ozlabs.org/project/ltp/patch/20240829114124.30299-1-wegao@suse.com/
> (where IMHO .needs_tmpdir = 1 is not needed due .mntpoint)
>
> $ cat > foo.c <<EOF
> #define MNTPOINT "mntpoint"
> #include "tst_test.h"
> 
> static void do_test(void)
> {
> 	tst_res(TPASS, "foo");
> }
> EOF
> 
> static struct tst_test test = {
> 	.test_all = do_test,
> 	.mntpoint = MNTPOINT,
> };
> 
> $ cd metadata; make metaparse && ./metaparse -I../include -I../testcases/kernel/syscalls/utils/ ../foo.c
> In file included from metaparse.c:17:
> In function ‘data_node_string’,
>     inlined from ‘main’ at metaparse.c:892:6:
> data_storage.h:84:20: warning: array subscript ‘struct data_node[0]’ is partly outside array bounds of ‘unsigned char[6]’ [-Warray-bounds=]
>    84 |         node->type = DATA_STRING;
>       |         ~~~~~~~~~~~^~~~~~~~~~~~~
> data_storage.h:79:34: note: object of size 6 allocated by ‘malloc’
>    79 |         struct data_node *node = malloc(size);
>       |                                  ^~~~~~~~~~~~

This seems to be one of the gcc bugs, where you shuffle code a little
bit and it triggers useless warnings. I guess that the compiler is
confused by the union usage. It goes away if you rewrite the code as:

diff --git a/metadata/data_storage.h b/metadata/data_storage.h
index 91ea70a02..cc9f9f233 100644
--- a/metadata/data_storage.h
+++ b/metadata/data_storage.h
@@ -81,8 +81,10 @@ static inline struct data_node *data_node_string(const char *string)
        if (!node)
                return NULL;

-       node->type = DATA_STRING;
-       strcpy(node->string.val, string);
+       struct data_node_string *str_node = &node->string;
+
+       str_node->type = DATA_STRING;
+       strcpy(str_node->val, string);

Also it seems that clang is smart enough not to produce the warnings.

> HOSTCC metadata/metaparse
>   "foo": {
>    "fname": ".../foo.c"
> 
>  metadata/metaparse.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/metadata/metaparse.c b/metadata/metaparse.c
> index 2384c73c86..24bf0f0968 100644
> --- a/metadata/metaparse.c
> +++ b/metadata/metaparse.c
> @@ -801,6 +801,7 @@ static struct implies {
>  		NULL}},
>  	{"all_filesystems", (const char *[]) {"needs_device", "needs_tmpdir",
>  		NULL}},
> +	{"mntpoint", (const char *[]) {"needs_tmpdir", NULL}},
>  	{"needs_device", (const char *[]) {"needs_tmpdir", NULL}},
>  	{"needs_checkpoints", (const char *[]) {"needs_tmpdir", NULL}},
>  	{"resource_files", (const char *[]) {"needs_tmpdir", NULL}},
> -- 
> 2.45.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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