Re: [LTP] [PATCH v4] save_restore: Introduce new struct field for flags

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +* 'TST_SR_TBROK_MISSING' – End test with 'TBROK' if the file does not exist
> +* 'TST_SR_TCONF_MISSING' – End test with 'TCONF' if the file does not exist
> +* 'TST_SR_SKIP_MISSING' – Continue without saving the file if it does not exist
> +* 'TST_SR_TBROK_RO' – End test with 'TBROK' if the file is read-only
> +* 'TST_SR_TCONF_RO' – End test with 'TCONF' if the file is read-only
> +* 'TST_SR_SKIP_RO' – Continue without saving the file if it is read-only
> +* 'TST_SR_IGNORE_ERR' – Ignore errors when writing new value into the file
> +
> +Common flag combinations also have shortcuts:
> +
> +* 'TST_SR_TCONF' – Equivalent to 'TST_SR_TCONF_MISSING | TST_SR_TCONF_RO'
> +* 'TST_SR_TBROK' – Equivalent to 'TST_SR_TBROK_MISSING | TST_SR_TBROK_RO'
> +* 'TST_SR_SKIP' – Equivalent to 'TST_SR_SKIP_MISSING | TST_SR_SKIP_RO'

This looks good now, great work!

>  'restore' is always strict and will TWARN if it encounters any error.
>  
>  [source,c]
>  -------------------------------------------------------------------------------
> -static void setup(void)
> -{
> -	FILE_PRINTF("/proc/sys/kernel/core_pattern", "/mypath");
> -	SAFE_TRY_FILE_PRINTF("/proc/sys/user/max_user_namespaces", "%d", 10);
> -}
> -
>  static struct tst_test test = {
>  	...
>  	.setup = setup,
>  	.save_restore = (const struct tst_path_val[]) {
> -		{"/proc/sys/kernel/core_pattern", NULL},
> -		{"?/proc/sys/user/max_user_namespaces", NULL},
> -		{"!/sys/kernel/mm/ksm/run", "1"},
> +		{"/proc/sys/kernel/core_pattern", NULL, TST_SR_TCONF},
> +		{"/proc/sys/user/max_user_namespaces", NULL, TST_SR_SKIP},
> +		{"/sys/kernel/mm/ksm/run", "1", TST_SR_TBROK},
>  		{}
>  	},
>  };
> diff --git a/include/tst_sys_conf.h b/include/tst_sys_conf.h
> index b7bbe36fc..4c85767be 100644
> --- a/include/tst_sys_conf.h
> +++ b/include/tst_sys_conf.h
> @@ -5,14 +5,26 @@
>  #ifndef TST_SYS_CONF_H__
>  #define TST_SYS_CONF_H__
>  
> +#define TST_SR_TCONF_MISSING 0x0
> +#define TST_SR_TBROK_MISSING 0x1
> +#define TST_SR_SKIP_MISSING 0x2
> +#define TST_SR_TCONF_RO 0x0
> +#define TST_SR_TBROK_RO 0x4
> +#define TST_SR_SKIP_RO 0x8
> +#define TST_SR_IGNORE_ERR 0x10
> +
> +#define TST_SR_TCONF (TST_SR_TCONF_MISSING | TST_SR_TCONF_RO)
> +#define TST_SR_TBROK (TST_SR_TBROK_MISSING | TST_SR_TBROK_RO)
> +#define TST_SR_SKIP (TST_SR_SKIP_MISSING | TST_SR_SKIP_RO)
> +
>  struct tst_path_val {
>          const char *path;
>          const char *val;
> +	unsigned int flags;
>  };
>  
> -int tst_sys_conf_save_str(const char *path, const char *value);
> -int tst_sys_conf_save(const char *path);
> -void tst_sys_conf_set(const char *path, const char *value);
> +void tst_sys_conf_save_str(const char *path, const char *value);
> +int tst_sys_conf_save(const struct tst_path_val *conf);
>  void tst_sys_conf_restore(int verbose);
>  void tst_sys_conf_dump(void);
>  
> diff --git a/lib/tst_sys_conf.c b/lib/tst_sys_conf.c
> index 003698825..78ed577e2 100644
> --- a/lib/tst_sys_conf.c
> +++ b/lib/tst_sys_conf.c
> @@ -20,6 +20,14 @@ struct tst_sys_conf {
>  
>  static struct tst_sys_conf *save_restore_data;
>  
> +static void print_error(int info_only, const char *err, const char *path)
> +{
> +	if (info_only)
> +		tst_res(TINFO | TERRNO, err, path);
> +	else
> +		tst_brk(TBROK | TERRNO, err, path);
> +}

If I remmeber correctly I did ask for this to become a macro or pass
__LINE__ so that we have a correct lines in the error output and you
haven't commented on that.

Otherwise the rest looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

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