[LTP] [PATCH 2/4] lib: enhance .save_restore to support set expected value
Li Wang
liwang@redhat.com
Wed Mar 9 03:27:52 CET 2022
Cyril Hrubis <chrubis@suse.cz> wrote:
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 9e745c537..fe2e2bb6c 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1105,11 +1105,12 @@ static void do_setup(int argc, char *argv[])
> > tst_tmpdir();
> >
> > if (tst_test->save_restore) {
> > - const char * const *name = tst_test->save_restore;
> > + const struct tst_path_val const *pvl =
> tst_test->save_restore;
> >
> > - while (*name) {
> > - tst_sys_conf_save(*name);
> > - name++;
> > + while (pvl->path) {
> > + if (!tst_sys_conf_save(pvl->path))
> > + tst_sys_conf_set(pvl->path, pvl->val);
>
> Maybe it would be cleaner if we added tst_sys_conf_save_set() function
> instead of tst_sys_conf_set() that would do both, saved the value and
> set new one if non-NULL.
>
Yes, it can be. Actually, I started writing like that at the beginning.
But I feel that tst_sys_conf_save_set() looks a bit longer and does more
things. To split the set into a separate function is to respect the UNIX
design
philosophy (one function does one thing). Another important reason
is to export tst_sys_conf_set() to global use instead of low-level macros
for knob settings.
The rest suggestions sound good to me, thanks for reviewing!
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220309/378ed5b3/attachment-0001.htm>
More information about the ltp
mailing list