[rds-devel] [External] : Re: [net-next, v2, 2/2] selftests: rds: Add -c config option to rds/config.sh

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
selftests: rds: Add -c config option to rds/config.sh

This patch adds a new -c flag to config.sh that enables callers to specify
the file path of the config they would like to update. If no config is
specified, the default will be the .config of the current directory.

> diff --git a/tools/testing/selftests/net/rds/config.sh b/tools/testing/selftests/net/rds/config.sh
> index 791c8dbe1095..e7a619d7cff0 100755
> --- a/tools/testing/selftests/net/rds/config.sh
> +++ b/tools/testing/selftests/net/rds/config.sh
> @@ -6,15 +6,20 @@ set -u
>  set -x
>
>  unset KBUILD_OUTPUT
> +CONF_FILE=""
> +FLAGS=""
>
>  GENERATE_GCOV_REPORT=0
> -while getopts "g" opt; do
> +while getopts "gc:" opt; do
>    case ${opt} in
>      g)
>        GENERATE_GCOV_REPORT=1
>        ;;
> +    c)
> +      CONF_FILE=$OPTARG
> +      ;;
>      :)
> -      echo "USAGE: config.sh [-g]"
> +      echo "USAGE: config.sh [-g] [-c config]"
>        exit 1
>        ;;
>      ?)
> @@ -24,30 +29,32 @@ while getopts "g" opt; do
>    esac
>  done
>
> -CONF_FILE="tools/testing/selftests/net/config"
> +if [[ "$CONF_FILE" != "" ]]; then
> +	FLAGS="--file $CONF_FILE"
> +fi

This changes the default behavior - config.sh now modifies .config in the
current directory when no -c option is provided, but the README.txt wasn't
updated. Looking at README.txt:

    # Create a suitable gcov enabled .config
    tools/testing/selftests/net/rds/config.sh -g

    # Alternatly create a gcov disabled .config
    tools/testing/selftests/net/rds/config.sh

    # build the kernel
    vng --build  --config tools/testing/selftests/net/config

Won't this workflow break? The old config.sh modified
tools/testing/selftests/net/config, but the new version modifies .config.
When following the README, config.sh now modifies .config, but vng builds
using the unmodified tools/testing/selftests/net/config, so the kernel
would be built with incorrect configuration (CONFIG_RDS, CONFIG_RDS_TCP,
CONFIG_NET_NS, CONFIG_VETH, CONFIG_NET_SCH_NETEM may not be properly
enabled, and CONFIG_MODULES may not be disabled).

Should the README examples be updated to either use
"-c tools/testing/selftests/net/config" or adjust the vng command to use
".config"?

Alternatively, for backward compatibility, the default could remain
tools/testing/selftests/net/config and require "-c .config" for the new
behavior.

>
>  # no modules
> -scripts/config --file "$CONF_FILE" --disable CONFIG_MODULES
> +scripts/config $FLAGS --disable CONFIG_MODULES

[ ... ]