Re: [LTP] [PATCH v2 1/3] lib: adding .arch field in tst_test structure

Cyril Hrubis
2021-11-04T10:25:51Z

Hi!
> > least array of supported architectures in the test_test structure would
> > be a good addition.
> >
>
> I guess defining .arch as a string and making a valid check will be enough.
> Array for that sounds a bit complicated in use.

Quite the opposite, it should be an array of strings, so that it's easy
to work with such as:

.supported_archs = (const char *const []){"x86_64", "ppc64le", NULL},

We can put it into a single string delimited by a space, but that would
be more complicated to work with.

> > However the hard part would be keeping the actual code and metadata in
> > sync, we still have to keep the ifdefs in the code.
> >
>
> Yes, some inline assemble require ifdefs.
>
> Btw, I look back at the reviews and find Jan said:
> "I can see how tst_on_arch() would be useful. Test is valid
> on all arches, but needs different input/constants/code/etc."
>
> That may be a slight reason for keeping tst_on_arch.

I guess that we should reviewe the code we have, I guess that there are
a few tests where we can get rid of a few ifdefs by doing the checks
dynamically.

Also I guess that it would be slightly easier to work with as an enum,
so that we can do:

switch (tst_arch) {
case TST_X86_64:
...
break;
case TST_PPC64_LE:
...
break;
default:
...
break;
}

instead of:
if (!strcmp(tst_arch, "x86_64"))
...
else if (!strmcp(tst_arch, ...)))
...
else
...

--
Cyril Hrubis
chrubis@suse.cz

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