Re: [RFC PATCH 1/4] check: refactor argument parsing with getopt

[Zorro Lang <zlang@kernel.org>]

On Fri, Aug 14, 2026 at 01:50:40PM +0530, Ojaswin Mujoo wrote:
> On Tue, May 12, 2026 at 09:25:36PM +0800, Zorro Lang wrote:
> > Replace the legacy, hand-written argument parsing loop with getopt.
> > Also compatible with old-style options (e.g. -nfs, -afs, -glusterfs,
> > -cifs, -9p, -fuse, -virtiofs, -pvfs2, -tmpfs, -ubifs, -overlay,
> > -udiff), pre-process them into long options before giving to getopt.
> > 
> > Signed-off-by: Zorro Lang <zlang@kernel.org>
> > ---
> >  check | 179 +++++++++++++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 126 insertions(+), 53 deletions(-)
> > 
> > diff --git a/check b/check
> > index ad685edc..11bdf81b 100755
> > --- a/check
> > +++ b/check
> > @@ -272,48 +272,97 @@ _prepare_test_list()
> >  	rm -f $tmp.list
> >  }
> >  
> > -# Process command arguments first.
> > +# Backward compatible with the old options mode. Translate word-style options
> > +# that getopt would misinterpret into long options.
> > +compat_old_option()
> > +{
> > +	check_args=()
> > +	while [ $# -gt 0 ]; do
> > +		case "$1" in
> > +		-nfs)		check_args+=("--fs" "nfs") ;;
> > +		-afs)		check_args+=("--fs" "afs") ;;
> > +		-glusterfs)	check_args+=("--fs" "glusterfs") ;;
> > +		-cifs)		check_args+=("--fs" "cifs") ;;
> > +		-9p)		check_args+=("--fs" "9p") ;;
> > +		-fuse)		check_args+=("--fs" "fuse") ;;
> > +		-virtiofs)	check_args+=("--fs" "virtiofs") ;;
> > +		-pvfs2)		check_args+=("--fs" "pvfs2") ;;
> > +		-tmpfs)		check_args+=("--fs" "tmpfs") ;;
> > +		-ubifs)		check_args+=("--fs" "ubifs") ;;
> > +		-overlay)	check_args+=("--fs" "overlay") ;;
> > +		-udiff)		check_args+=("--udiff") ;;
> > +		*)		check_args+=("$1") ;;
> > +		esac
> > +		shift
> > +	done
> > +}
> > +
> > +short_opts="g:x:X:e:E:s:S:lnri:I:TdbR:L:h"
> > +long_opts="fs:,exact-order,large-fs,extra-space:,udiff,help"
> > +
> > +compat_old_option "$@"
> > +
> > +# Note: The '+' prefix in getopt's option string preserves the existing
> > +# behavior that option parsing stops at the first non-option test argument.
> > +parsed_opts=$(getopt -n "check" -o +"${short_opts}" -l "$long_opts" -- "${check_args[@]}")
> > +test $? -ne 0 && usage
> > +
> > +eval set -- "$parsed_opts"
> > +
> >  while [ $# -gt 0 ]; do
> >  	case "$1" in
> > -	-\? | -h | --help) usage ;;
> 
> Hi Zorro, mostly looks good but I think we slightly change the behavior
> here. Earlier ./check -\? would print help but not:
> 
>    ./check -\?
>   check: invalid option -- '?'
>   Warning: need to define parameters for host <host>
>          or set variables:
>           TEST_DIR TEST_DEV
> 
> Maybe we should add this to compat_old_option ( although i really doubt
> anyone would care about -\? :) )

Thanks for the review! Turns out `-\?` is indeed an option

  while [ $# -gt 0 ]; do
        case "$1" in
        -\? | -h | --help) usage ;;

I've never used it before and wasn't sure if anyone actually does. But given
that it's the existing behavior, let's keep it consistent. I'll update and
remove the "RFC" flag in next version.

Thanks,
Zorro

> 
> Other than that, feel free to add:
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > -
> > -	-nfs|-afs|-glusterfs|-cifs|-9p|-fuse|-virtiofs|-pvfs2|-tmpfs|-ubifs)
> > -		FSTYP="${1:1}"
> > +	--fs)
> > +		if [ "$2" == "overlay" ];then
> > +			[ "$FSTYP" == overlay ] || \
> > +				export OVL_BASE_FSTYP="$FSTYP"
> > +			FSTYP=overlay
> > +			export OVERLAY=true
> > +		else
> > +			FSTYP="$2"
> > +		fi
> > +		shift
> >  		;;
> > -	-overlay)
> > -		[ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> > -		FSTYP=overlay
> > -		export OVERLAY=true
> > +	--udiff)
> > +		diff="$diff -u"
> >  		;;
> > -
> > -	-g)	group=$2 ; shift ;
> > -		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> > +	-g)
> > +		GROUP_LIST="$GROUP_LIST ${2//,/ }"
> > +		shift
> >  		;;
> > -
> > -	-x)	xgroup=$2 ; shift ;
> > -		XGROUP_LIST="$XGROUP_LIST ${xgroup//,/ }"
> > +	-x)
> > +		XGROUP_LIST="$XGROUP_LIST ${2//,/ }"
> > +		shift
> >  		;;
> > -
> > -	-X)	subdir_xfile=$2; shift ;
> > +	-X)
> > +		subdir_xfile="$2"
> > +		shift
> >  		;;
> >  	-e)
> > -		xfile=$2; shift ;
> >  		readarray -t -O "${#exclude_tests[@]}" exclude_tests < \
> > -			<(echo "$xfile" | tr ', ' '\n\n')
> > +			<(echo "$2" | tr ', ' '\n\n')
> > +		shift
> >  		;;
> > -
> > -	-E)	xfile=$2; shift ;
> > -		if [ -f $xfile ]; then
> > +	-E)
> > +		if [ -f "$2" ]; then
> >  			readarray -t -O ${#exclude_tests[@]} exclude_tests < \
> > -				<(sed "s/#.*$//" $xfile)
> > +				<(sed "s/#.*$//" "$2")
> >  		fi
> > +		shift
> > +		;;
> > +	-s)
> > +		RUN_SECTION="$RUN_SECTION $2"
> > +		shift
> > +		;;
> > +	-S)
> > +		EXCLUDE_SECTION="$EXCLUDE_SECTION $2"
> > +		shift
> > +		;;
> > +	-l)
> > +		diff="diff"
> > +		;;
> > +	-n)
> > +		showme=true
> >  		;;
> > -	-s)	RUN_SECTION="$RUN_SECTION $2"; shift ;;
> > -	-S)	EXCLUDE_SECTION="$EXCLUDE_SECTION $2"; shift ;;
> > -	-l)	diff="diff" ;;
> > -	-udiff)	diff="$diff -u" ;;
> > -
> > -	-n)	showme=true ;;
> >  	-r)
> >  		if $exact_order; then
> >  			_fatal "Cannot specify -r and --exact-order."
> > @@ -322,40 +371,64 @@ while [ $# -gt 0 ]; do
> >  		;;
> >  	--exact-order)
> >  		if $randomize; then
> > -			_fatal "Cannnot specify --exact-order and -r."
> > +			_fatal "Cannot specify --exact-order and -r."
> >  		fi
> >  		exact_order=true
> >  		;;
> > -	-i)	iterations=$2; shift ;;
> > -	-I) 	iterations=$2; istop=true; shift ;;
> > -	-T)	timestamp=true ;;
> > -	-d)	DUMP_OUTPUT=true ;;
> > -	-b)	brief_test_summary=true;;
> > -	-R)	report_fmt=$2 ; shift ;
> > -		REPORT_LIST="$REPORT_LIST ${report_fmt//,/ }"
> > +	-i)
> > +		iterations=$2
> > +		shift
> > +		;;
> > +	-I)
> > +		iterations=$2
> > +		istop=true
> > +		shift
> > +		;;
> > +	-T)
> > +		timestamp=true
> > +		;;
> > +	-d)
> > +		DUMP_OUTPUT=true
> > +		;;
> > +	-b)
> > +		brief_test_summary=true
> > +		;;
> > +	-R)
> > +		REPORT_LIST="$REPORT_LIST ${2//,/ }"
> >  		do_report=true
> > +		shift
> >  		;;
> > -	--large-fs) export LARGE_SCRATCH_DEV=yes ;;
> > -	--extra-space=*) export SCRATCH_DEV_EMPTY_SPACE=${r#*=} ;;
> > -	-L)	[[ $2 =~ ^[0-9]+$ ]] || usage
> > -		loop_on_fail=$2; shift
> > +	--large-fs)
> > +		export LARGE_SCRATCH_DEV=yes
> > +		;;
> > +	--extra-space)
> > +		export SCRATCH_DEV_EMPTY_SPACE="$2"
> > +		shift
> > +		;;
> > +	-L)
> > +		[[ $2 =~ ^[0-9]+$ ]] || usage
> > +		loop_on_fail=$2
> > +		shift
> > +		;;
> > +	-h|--help)
> > +		usage
> > +		;;
> > +	--)
> > +		shift
> > +		break
> > +		;;
> > +	*)
> > +		usage
> >  		;;
> > -
> > -	-*)	usage ;;
> > -	*)	# not an argument, we've got tests now.
> > -		have_test_arg=true ;;
> >  	esac
> > -
> > -	# if we've found a test specification, the break out of the processing
> > -	# loop before we shift the arguments so that this is the first argument
> > -	# that we process in the test arg loop below.
> > -	if $have_test_arg; then
> > -		break;
> > -	fi
> > -
> >  	shift
> >  done
> >  
> > +# Remaining arguments
> > +if [ $# -gt 0 ]; then
> > +	have_test_arg=true
> > +fi
> > +
> >  # we need common/rc, that also sources common/config. We need to source it
> >  # after processing args, overlay needs FSTYP set before sourcing common/config
> >  if ! . ./common/rc; then
> > -- 
> > 2.54.0
> >