Re: [LTP] [PATCH v4 11/19] controllers: Expand cgroup_lib shell library

[Petr Vorel <pvorel@suse.cz>]

Hi Luke,

> diff --git a/testcases/kernel/controllers/cgroup_lib.sh b/testcases/kernel/controllers/cgroup_lib.sh
...
> +_cgroup_state=
> +
> +_cgroup_check_return()
> +{
> +	local ret="$1"
> +	local msg="$2"
> +
> +	tst_flag2mask TBROK
> +	[ "$ret" = "$?" ] && tst_brk TBROK "$msg"
> +
> +	tst_flag2mask TCONF
> +	[ "$ret" = "$?" ] && tst_brk TCONF "$msg"
> +}
As I wrote in previous patch likely we can avoid tst_flag2mask in new API.

In few cases where needed we hardwired numbers (IMHO POSIX shell does not
support constants, which would be better than variables which can be changed).

In this case you could probably use ROD() or EXPECT_PASS_BRK().
We use ROD() for external commands, thus support only TBROK on 0,
but it should be probably safe to add TCONF on 32. If others consider it
dangerous, we can add another function in tst_test.sh, e.g. ROD_LTP() which
would expect LTP exit codes.

...
> +# Cleans up any setup done by calling cgroup_require.
> +# USAGE: cgroup_cleanup
> +# Can be safely called even when no setup has been done
> +cgroup_cleanup()
> +{
> +	[ "$_cgroup_state" = "" ] && return 0
> +
> +	tst_cgctl cleanup "$_cgroup_state"
> +
> +	_cgroup_check_return "$?" "cgroup_cleanup: tst_cgctl cleanup exited"
> +
> +	_cgroup_state=""
nit: can be just
_cgroup_state=


Kind regards,
Petr

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