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

Richard Palethorpe
2021-11-05T10:00:21Z

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> 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:

I prefer enum as well. As an aside, we don't want to include LE in
ppc64. If someone finds that the byte order is significant for a test
then we can add ppc64le or ppc64be. Also at some point we may need to
add a "machine" field for e.g. POWER8, i386 etc.

Which btw, I have some buildroot and QEMU scripts which can be used to
test ppc64 BE and any other machine you have the hardware or QEMU
emulator for.

https://gitlab.com/Palethorpe/cross

> ...
> break;
> default:
> ...
> break;
> }
>
> instead of:
> if (!strcmp(tst_arch, "x86_64"))
> ...
> else if (!strmcp(tst_arch, ...)))
> ...
> else
> ...

--
Thank you,
Richard.

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