Re: [LTP] [PATCH v2 09/18] testcases/lib: Implement tst_cgctl binary

[Petr Vorel <pvorel@suse.cz>]

> Implement a binary utility that creates an interface to make calls to
> the cgroup C API.

> This will effectively allow shell scripts to make calls to the cgroup C
> api.

> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
> v2: Add license identifier and copyright.
> Reformat with tabs instead of spaces.
> Add help format message and help function.
> Add error gotos to streamline error messaging.

>  testcases/lib/Makefile    |  2 +-
>  testcases/lib/tst_cgctl.c | 87 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 1 deletion(-)
>  create mode 100644 testcases/lib/tst_cgctl.c

> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index f2de0c832..f4f8c8524 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -12,6 +12,6 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
>  			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars\
>  			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
>  			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
> -			   tst_check_kconfigs
> +			   tst_check_kconfigs tst_cgctl

>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/lib/tst_cgctl.c b/testcases/lib/tst_cgctl.c
> new file mode 100644
> index 000000000..4f4fe8542
> --- /dev/null
> +++ b/testcases/lib/tst_cgctl.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Canonical Ltd.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include "tst_cgroup.h"
> +
> +#define USAGE "Usage: tst_cgctl require [controller] [test_pid]\n\
> +	\t cleanup [config (output of tst_cg_print_config)]\n\
> +	\t print\n\
> +	\t help\n"
nit: instead of define I'd create function usage().
I'd print always to stderr, just exit code would be different.
(we print tst_res/tst_brk output to stderr, thus it's better to use it).

Also checkpatch does not like it:
$ cd testcases/lib/ && make check-tst_cgctl
CHECK testcases/lib/tst_cgctl.c
tst_cgctl.c:13: WARNING: Avoid line continuations in quoted strings

The rest LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

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