Re: [LTP] [PATCH v6 02/10] controllers: Expand cgroup_lib shell library

[Petr Vorel <pvorel@suse.cz>]

Hi Luke,

> v5->v6: Added $ret to capture return code and added return $ret
> 	after tst_brk calls.

>  testcases/kernel/controllers/cgroup_lib.sh | 137 ++++++++++++++++++---
...
> +# Mounts and configures the given controller
> +# USAGE: cgroup_require CONTROLLER
> +cgroup_require()
> +{
> +	local ctrl="$1"
> +	local ret
> +
> +	[ $# -eq 0 ] && tst_brk TBROK "cgroup_require: controller not defined"
> +
> +	[ ! -f /proc/cgroups ] && tst_brk TCONF "Kernel does not support control groups"
> +
> +	_cgroup_state=$(tst_cgctl require "$ctrl" $$)
> +	ret=$?
> +
> +	if [ $ret -eq 32 ]; then
> +		tst_brk TCONF "'tst_cgctl require' exited. Controller is probably not available?"
> +		return $ret
FYI I merged this part as is, but I'd personally avoid using return in TCONF:
1) return value is not anywhere used, thus plain return would be enough.
2) Keeping $? in $ret helps to avoid first TBROK ("'tst_cgctl require' exited") and
second TBROK will not be presented due $_cgroup_state being empty on error (you
print all errors to stderr.
My concern was just not to overwrite $?

Kind regards,
Petr

> +	fi
> +
> +	if [ $ret -ne 0 ]; then
> +		tst_brk TBROK "'tst_cgctl require' exited."
> +		return $ret
> +	fi
> +
> +	[ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_require: No state was set after call to tst_cgctl require?"
> +
> +	return 0
>  }

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