[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