Re: [LTP] [PATCH] lib: rewrite cgroup_find_ctrl with using for_each_ctrl

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello Li,

Li Wang <liwang@redhat.com> writes:

> It is safe to start from controllers[0] to traverse each of
> the controller whatever V2 or V1, then we can make use of it
> in the cgroup_find_ctrl() function.

Right, it seems we never set ctrl_root on "cgroup" nor is it added to
ctrl_field. So it will be skipped in other loops. This might not be what
people expect, but I'm not sure what to do about that.

>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  lib/tst_cgroup.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 2ef599d9e..10b65364b 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -214,7 +214,7 @@ static const char *cgroup_v2_ltp_mount = "unified";
>  #define for_each_v1_root(r)			\
>  	for ((r) = roots + 1; (r)->ver; (r)++)
>  #define for_each_ctrl(ctrl)			\
> -	for ((ctrl) = controllers + 1; (ctrl)->ctrl_name; (ctrl)++)
> +	for ((ctrl) = controllers; (ctrl)->ctrl_name; (ctrl)++)
>  
>  /* In all cases except one, this only loops once.
>   *
> @@ -325,15 +325,14 @@ void tst_cgroup_print_config(void)
>  __attribute__ ((nonnull, warn_unused_result))
>  static struct cgroup_ctrl *cgroup_find_ctrl(const char *const ctrl_name)
>  {
> -	struct cgroup_ctrl *ctrl = controllers;
> -
> -	while (ctrl->ctrl_name && strcmp(ctrl_name, ctrl->ctrl_name))
> -		ctrl++;
> +	struct cgroup_ctrl *ctrl;
>  
> -	if (!ctrl->ctrl_name)
> -		ctrl = NULL;
> +	for_each_ctrl(ctrl) {
> +		if (!strcmp(ctrl_name, ctrl->ctrl_name))
> +			return ctrl;
> +	}
>  
> -	return ctrl;
> +	return NULL;
>  }
>  
>  /* Determine if a mounted cgroup hierarchy is unique and record it if so.

Nice simplification!

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

-- 
Thank you,
Richard.

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