Re: [PATCH v6] strace-tst-thp.sh: Allow unsupported THP tests

On 21/07/26 05:34, H.J. Lu wrote:
> Change strace-tst-thp.sh to check the command exit status so that
> unsupported THP tests exit with status 77.
> 
> 
> From 664b271b81da6d57eefac7cd7d6b529dfbf5755c Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <[email protected]>
> Date: Sat, 4 Jul 2026 11:06:21 +0800
> Subject: [PATCH v6] strace-tst-thp.sh: Allow unsupported THP tests
> 
> Change strace-tst-thp.sh to check the command exit status so that
> unsupported THP tests exit with status 77.
> 
> Signed-off-by: H.J. Lu <[email protected]>

LGTM, thanks.  Some minor suggestions below, no need to send a new version.

Reviewed-by: Adhemerval Zanella  <[email protected]>

> ---
>  sysdeps/unix/sysv/linux/strace-tst-thp.sh | 24 +++++++++++------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/strace-tst-thp.sh b/sysdeps/unix/sysv/linux/strace-tst-thp.sh
> index bff7d3e7a0..1c2b7255ac 100644
> --- a/sysdeps/unix/sysv/linux/strace-tst-thp.sh
> +++ b/sysdeps/unix/sysv/linux/strace-tst-thp.sh
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/bin/sh
>  # Run THP test under strace to verify control of the THP segment load.
>  # Copyright (C) 2026 Free Software Foundation, Inc.
>  # This file is part of the GNU C Library.
> @@ -17,8 +17,6 @@
>  # License along with the GNU C Library; if not, see
>  # <https://www.gnu.org/licenses/>.
>  
> -set -e
> -
>  rtld="$1"
>  test_wrapper_env="$2"
>  run_program_env="$3"
> @@ -39,24 +37,24 @@ esac
>  
>  # Verify strace is not just present, but works in this environment.  If
>  # not, skip the test.
> -/bin/sh -c \
> - "${test_wrapper_env} ${run_program_env} \
> -  strace -X raw -e trace=none -- /bin/true" > /dev/null 2>&1 || exit 77
> +${test_wrapper_env} ${run_program_env} \
> +  strace -X raw -e trace=none -- /bin/true > /dev/null 2>&1 || exit 77
>  
>  # Finally the actual test inside the test environment, using the just
>  # build ld.so and new libraries to run the THP test under strace.
> -if /bin/sh -c \
> -  "timeout -k 4 $((3*$TIMEOUTFACTOR)) ${cmd} --direct 2>&1 \
> -   | grep -E \"madvise\(0x[0-9a-f]+, [0-9]+, 0xe)\""; then
> +output=$(timeout -k 4 $((3*$TIMEOUTFACTOR)) ${cmd} --direct 2>&1)
> +test $? = 77 && exit 77
> +if echo "${output}" | grep -E "madvise\(0x[0-9a-f]+, [0-9]+, 0xe)"; then

I think maybe it would be better to use 'printf '%s\n' "${output}"' to avoid
some shell like dash to avoid interprets backslash escapes (since this is a
strace output and we might have some eventually).

>    if test ${strace_expected} = yes; then
> -    exit 0
> +    status=0
>    else
> -    exit 1
> +    status=1
>    fi
>  else
>    if test ${strace_expected} = no; then
> -    exit 0
> +    status=0
>    else
> -    exit 1
> +    status=1
>    fi
>  fi
> +exit ${status}
> -- 
> 2.55.0
>