Re: [LTP] [PATCH v3] lib: Introduce tst_path_macros.h to consolidate system paths

[Li Wang <li.wang@linux.dev>]

Petr Vorel wrote:
 
> The problem is even now on master:
> output of testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c contains e.g. PATH_OC_HPAGES
> from include/tst_hugepage.h.
> 
> https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html#hugemmap10
> 
> da3088183a ("metadata: metaparse: Implement recursive include")
> skips certain LTP includes e.g. tst_test.h in skip_includes[] causes
> all definitions not being propagated. I guess it could be fixable to parse
> include/tst_test.h and include certain headers.
> 
> $ metadata/metaparse -v testcases/kernel/mem/hugetlb/hugemmap/hugemmap10.c
> does not print INCLUDE .. because include/tst_hugepage.h is included via
> tst_test.h, not directly in the test.

After looking into the mataparse.c I think we have two methods:

1. Explicitly parsing tst_path_defs.h in metaparse.c:

   Since tst_test.h is explicitly ignored by the skip_includes[] array,
   any files it includes (e.g. tst_path_defs.h) are consequently bypassed
   by the parser. To resolve this, we can instruct metaparse to proactively
   search for and parse tst_path_defs.h during its initialization phase.

   We can achieve this by adding a dedicated function in metaparse.c to
   read this specific header file, and then invoking it within the main
   parse_file function.


2. Simply adding the list of macros to the internal_macros[].

   As long as all the konbs definitions are listed there, metaparse can
   convert them into string paths in tst_test for use in ltp.json.

--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -881,6 +881,66 @@ static const struct macro {
        {"TST_SR_TBROK_RO", "TBROK_RO"},
        {"TST_SR_TCONF_RO", "TCONF_RO"},
        {"TST_SR_SKIP_RO", "SKIP_RO"},
+        {"PATH_KERNEL_HOSTNAME", "/proc/sys/kernel/hostname"},
+        {"PATH_KERNEL_OSRELEASE", "/proc/sys/kernel/osrelease"},
+        {"PATH_KERNEL_VERSION", "/proc/sys/kernel/version"},
+        {"PATH_KERNEL_DOMAINNAME", "/proc/sys/kernel/domainname"},
+        {"PATH_KERNEL_PRINTK", "/proc/sys/kernel/printk"},
+        {"PATH_KERNEL_PID_MAX", "/proc/sys/kernel/pid_max"},
+        {"PATH_KERNEL_SHMMAX", "/proc/sys/kernel/shmmax"},
...

I don't have any preferences, but method-1 looks more elegant.

> My fear was that to clash with existing system headers causing build failure due
> macro redefinition (that's IMHO reason for "TST_" prefix). But "PATH_" might
> be safe, because /usr/include/paths.h contains "_PATH_" prefix - with leading
> underscore (e.g. _PATH_CONSOLE). And the only "PATH_" prefix is PATH_MAX
> in /usr/include/limits.h.

We now have PATH_{KERNEL|USER|VM|FS}, so conflicts should be very low.

-- 
Regards,
Li Wang

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