[LTP] [PATCH 3/3] lib/tst_kconfig: Make use of boolean expression eval
Richard Palethorpe
rpalethorpe@suse.de
Thu Oct 22 10:38:14 CEST 2020
Hello,
I don't see anything obviously wrong here, just some nits.
Cyril Hrubis <chrubis@suse.cz> writes:
> Now each string in the kconfig[] array in tst_test structure is an
> boolean expression which is evaluated. All expressions has to be true in
> order for the test to continue.
>
> + Update the docs.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> CC: Pengfei Xu <pengfei.xu@intel.com>
> ---
> doc/test-writing-guidelines.txt | 21 ++--
> lib/newlib_tests/test_kconfig.c | 1 +
> lib/tst_kconfig.c | 186 ++++++++++++++++++++++++--------
> 3 files changed, 154 insertions(+), 54 deletions(-)
>
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index 1a51ef7c7..3c2ab7166 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -1643,21 +1643,26 @@ on the system, disabled syscalls can be detected by checking for 'ENOSYS'
> errno etc.
>
> However in rare cases core kernel features couldn't be detected based on the
> -kernel userspace API and we have to resort to kernel .config parsing.
> +kernel userspace API and we have to resort to parse the kernel .config.
>
> -For this cases the test should set the 'NULL' terminated '.needs_kconfigs' array
> -of kernel config options required for the test. The config option can be
> -specified either as plain "CONFIG_FOO" in which case it's sufficient for the
> -test continue if it's set to any value (typically =y or =m). Or with a value
> -as "CONFIG_FOO=bar" in which case the value has to match as well. The test is
> -aborted with 'TCONF' if any of the required options were not set.
> +For this cases the test should set the 'NULL' terminated '.needs_kconfigs'
> +array of boolean expressions with constraints on the kconfig variables. The
> +boolean expression consits of variables, two binary operations '&' and '|',
> +negation '!' and correct sequence of parentesis '()'. Variables are expected
> +to be in a form of "CONFIG_FOO[=bar]".
> +
> +The test will continue to run if all expressions are evaluated to 'True'.
> +Missing variable is mapped to 'False' as well as variable with different than
> +specified value, e.g. 'CONFIG_FOO=bar' will evaluate to 'False' if the value
> +is anything else but 'bar'. If config variable is specified as plain
> +'CONFIG_FOO' it's evaluated to true it's set to any value (typically =y or =m).
>
> [source,c]
> -------------------------------------------------------------------------------
> #include "tst_test.h"
>
> static const char *kconfigs[] = {
> - "CONFIG_X86_INTEL_UMIP",
> + "CONFIG_X86_INTEL_UMIP | CONFIG_X86_UMIP",
> NULL
> };
>
> diff --git a/lib/newlib_tests/test_kconfig.c b/lib/newlib_tests/test_kconfig.c
> index d9c662fc5..183d55611 100644
> --- a/lib/newlib_tests/test_kconfig.c
> +++ b/lib/newlib_tests/test_kconfig.c
> @@ -14,6 +14,7 @@ static const char *kconfigs[] = {
> "CONFIG_MMU",
> "CONFIG_EXT4_FS=m",
> "CONFIG_PGTABLE_LEVELS=4",
> + "CONFIG_MMU & CONFIG_EXT4_FS=m",
> NULL
> };
>
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index f80925cc9..cd99a3034 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -12,6 +12,7 @@
> #define TST_NO_DEFAULT_MAIN
> #include "tst_test.h"
> #include "tst_kconfig.h"
> +#include "tst_bool_expr.h"
>
> static const char *kconfig_path(char *path_buf, size_t path_buf_len)
> {
> @@ -184,86 +185,179 @@ static size_t array_len(const char *const kconfigs[])
> return i;
> }
>
> -static int compare_res(struct tst_kconfig_var *var, const char *kconfig,
> - char choice, const char *val)
> +static inline unsigned int get_len(const char* kconfig)
> {
> - if (var->choice != choice) {
> - tst_res(TINFO, "Needs kernel %s, have %c", kconfig, var->choice);
> - return 1;
> - }
> + char *sep = index(kconfig, '=');
>
> - if (choice != 'v')
> - return 0;
> + if (!sep)
> + return strlen(kconfig);
>
> - if (strcmp(var->val, val)) {
> - tst_res(TINFO, "Needs kernel %s, have %s", kconfig, var->val);
> - return 1;
> + return sep - kconfig;
> +}
> +
> +static inline unsigned int get_var_cnt(struct tst_expr *exprs[],
const *?
> + unsigned int expr_cnt)
> +{
> + unsigned int i;
> + struct tst_expr *j;
> + unsigned int cnt = 0;
> +
> + for (i = 0; i < expr_cnt; i++) {
> + for (j = exprs[i]; j; j = j->next) {
> + if (j->op == TST_OP_VAR)
> + cnt++;
> + }
> }
>
> - return 0;
> + return cnt;
> }
>
> -static inline unsigned int get_len(const char* kconfig)
> +static struct tst_kconfig_var *find_var(struct tst_kconfig_var
> vars[],
also const []?
> + unsigned int var_cnt,
> + const char *var)
> {
> - char *sep = index(kconfig, '=');
> + unsigned int i;
>
> - if (!sep)
> - return strlen(kconfig);
> + for (i = 0; i < var_cnt; i++) {
> + if (!strcmp(vars[i].id, var))
> + return &vars[i];
> + }
>
> - return sep - kconfig;
> + return NULL;
> }
>
> -void tst_kconfig_check(const char *const kconfigs[])
> +/*
> + * Fill in the kconfig variables array from the expressions. Also makes sure
> + * that each variable is copied to the array exaclty once.
> + */
> +static inline unsigned int get_vars(struct tst_expr *exprs[],
> + unsigned int expr_cnt,
> + struct tst_kconfig_var vars[])
get_vars sounds like a read-onlyl operation, maybe populate_vars or similar?
> {
> - size_t vars_cnt = array_len(kconfigs);
> - struct tst_kconfig_var vars[vars_cnt];
> unsigned int i;
> - int abort_test = 0;
> + struct tst_expr *j;
> + unsigned int cnt = 0;
> +
> + for (i = 0; i < expr_cnt; i++) {
> + for (j = exprs[i]; j; j = j->next) {
> + struct tst_kconfig_var *var;
>
> - memset(vars, 0, sizeof(*vars) * vars_cnt);
> + if (j->op != TST_OP_VAR)
> + continue;
>
> - for (i = 0; i < vars_cnt; i++) {
> - vars[i].id_len = get_len(kconfigs[i]);
> + vars[cnt].id_len = get_len(j->val);
>
> - if (vars[i].id_len >= sizeof(vars[i].id))
> - tst_brk(TBROK, "kconfig var id too long!");
> + if (vars[cnt].id_len >= sizeof(vars[cnt].id))
> + tst_brk(TBROK, "kconfig var id too long!");
>
> - strncpy(vars[i].id, kconfigs[i], vars[i].id_len);
> + strncpy(vars[cnt].id, j->val, vars[cnt].id_len);
> +
> + var = find_var(vars, cnt, vars[cnt].id);
> +
> + if (var)
> + j->priv = var;
> + else
> + j->priv = &vars[cnt++];
> + }
> }
>
> - tst_kconfig_read(vars, vars_cnt);
> + return cnt;
> +}
> +
> +static int map(struct tst_expr *expr)
> +{
> + struct tst_kconfig_var *var = expr->priv;
>
> - for (i = 0; i < vars_cnt; i++) {
> - if (vars[i].choice == 0) {
> - tst_res(TINFO, "Missing kernel %s", kconfigs[i]);
> - abort_test = 1;
> + if (var->choice == 0)
> + return 0;
> +
> + const char *val = strchr(expr->val, '=');
> +
> + /* CONFIG_FOO evaluates to true if y or m */
> + if (!val)
> + return var->choice == 'y' || var->choice == 'm';
> +
> + char choice = 'v';
> + val++;
> +
> + if (!strcmp(val, "n"))
> + choice = 'n';
> +
> + if (!strcmp(val, "y"))
> + choice = 'y';
> +
> + if (!strcmp(val, "m"))
> + choice = 'm';
> +
> + if (choice != 'v')
> + return var->choice == choice;
> +
> + return !strcmp(val, var->val);
> +}
> +
> +static void dump_vars(struct tst_expr *expr)
const *?
> +{
> + struct tst_expr *i;
> + struct tst_kconfig_var *var;
> +
> + tst_res(TINFO, "Variables:");
> +
> + for (i = expr; i; i = i->next) {
> + if (i->op != TST_OP_VAR)
> + continue;
> +
> + var = i->priv;
> +
> + if (!var->choice) {
> + tst_res(TINFO, "%s Undefined", var->id);
> continue;
> }
>
> - if (vars[i].choice == 'n') {
> - tst_res(TINFO, "Kernel %s is not set", kconfigs[i]);
> - abort_test = 1;
> + if (var->choice == 'v') {
> + tst_res(TINFO, "%s = %s", var->id, var->val);
> continue;
> }
>
> - const char *val = strchr(kconfigs[i], '=');
> + tst_res(TINFO, "%s = %c", var->id, var->choice);
> + }
> +}
>
> - if (val) {
> - char choice = 'v';
> - val++;
> +void tst_kconfig_check(const char *const kconfigs[])
> +{
> + size_t expr_cnt = array_len(kconfigs);
> + struct tst_expr *exprs[expr_cnt];
> + unsigned int i, var_cnt;
> + int abort_test = 0;
> +
> + for (i = 0; i < expr_cnt; i++) {
> + exprs[i] = tst_bool_expr_parse(kconfigs[i]);
> +
> + if (!exprs[i])
> + tst_brk(TBROK, "Invalid kconfig expression!");
> + }
>
> - if (!strcmp(val, "y"))
> - choice = 'y';
> + var_cnt = get_var_cnt(exprs, expr_cnt);
> + struct tst_kconfig_var vars[var_cnt];
>
> - if (!strcmp(val, "m"))
> - choice = 'm';
> + var_cnt = get_vars(exprs, expr_cnt, vars);
>
> - if (compare_res(&vars[i], kconfigs[i], choice, val))
> - abort_test = 1;
> + tst_kconfig_read(vars, var_cnt);
>
> + for (i = 0; i < expr_cnt; i++) {
> + int val = tst_bool_expr_eval(exprs[i], map);
> +
> + if (val != 1) {
> + abort_test = 1;
> + tst_res(TINFO, "Expression '%s' not satisfied!", kconfigs[i]);
> + dump_vars(exprs[i]);
> }
>
> - free(vars[i].val);
> + tst_bool_expr_free(exprs[i]);
> + }
> +
> + for (i = 0; i < var_cnt; i++) {
> + if (vars[i].choice == 'v')
> + free(vars[i].val);
> }
>
> if (abort_test)
> --
> 2.26.2
--
Thank you,
Richard.
More information about the ltp
mailing list