Re: [LTP] [PATCH v2 6/6] mem/vma05.sh: Convert to the new shell library

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Thanks for these improvements. Few notes below.

> To run the test from the test directory do:

> PATH=$PATH:$PWD/../../../lib/:$PWD/testcases/lib/:: ./vma05.sh
very nit: safer example would be to have $PATH at the end of the list.

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  testcases/kernel/mem/vma/vma05.sh | 96 ++++++++++++++++---------------
>  1 file changed, 49 insertions(+), 47 deletions(-)

> diff --git a/testcases/kernel/mem/vma/vma05.sh b/testcases/kernel/mem/vma/vma05.sh
> index e1ef1014e..1876d7663 100755
> --- a/testcases/kernel/mem/vma/vma05.sh
> +++ b/testcases/kernel/mem/vma/vma05.sh
> @@ -1,6 +1,15 @@
>  #!/bin/sh
> +#
>  # SPDX-License-Identifier: GPL-2.0-or-later
> +#
very nit: I would not add these new lines.

>  # Copyright (C) 2017 Red Hat, Inc.
> +# Copyright (C) 2024 Cyril Hrubis <chrubis@suse.cz>
> +#
> +# ---
> +# doc
> +#
> +# [Description]
> +#
>  # Regression test if the vsyscall and vdso VMA regions are reported correctly.

>  # While [vsyscall] is mostly deprecated with newer systems, there is
> @@ -15,58 +24,51 @@
>  # VM_ALWAYSDUMP)). As a consequence of this bug, VMAs were not included
>  # in core dumps which resulted in eg. incomplete backtraces and invalid
>  # core dump files created by gdb.
> +# ---
> +#
> +# ---
> +# env
> +# {
> +#  "needs_root": true,
> +#  "needs_tmpdir": true,
> +#  "needs_cmds": ["gdb"],
Maybe also "uname"?

> +#  "save_restore": [
> +#   ["/proc/sys/kernel/core_pattern", "core", "TBROK"],
> +#   ["/proc/sys/kernel/core_uses_pid", "0", "TBROK"]
> +#  ],
> +#  "tags": [
> +#   ["linux-git", "103efcd9aac1"],
> +#   ["linux-git", "b6558c4a2378"],
> +#   ["linux-git", "e5b97dde514f"]

I wonder how hard to add shell support (with help of ujson) to metadata/metaparse.c.

I created a ticket for it:
https://github.com/linux-test-project/ltp/issues/1216

> +#  ]
> +# }
> +# ---

> -TST_SETUP=setup
> -TST_CLEANUP=cleanup


> -TST_TESTFUNC=vma_report_check
> -TST_NEEDS_ROOT=1
> -TST_NEEDS_TMPDIR=1
> -TST_NEEDS_CMDS="gdb"
> -
> -CORE_LIMIT=$(ulimit -c)
> -CORE_PATTERN=$(cat /proc/sys/kernel/core_pattern)
> -CORE_USES_PID=$(cat /proc/sys/kernel/core_uses_pid)
> -
> -setup()
> -{
> -	ulimit -c unlimited
> -	echo "core" > /proc/sys/kernel/core_pattern
> -	echo 0 > /proc/sys/kernel/core_uses_pid
> -	unset DEBUGINFOD_URLS
> -}
We don't need setup and cleanup at all. But I see we don't have a support for
them in testcases/lib/tst_run_shell.c yet. I might need to add it for
netstress.c.

...

FYI testcases/lib/tests/shell_loader*.sh would deserve SPDX and copyright.

Kind regards,
Petr

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