Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()

[Philippe Mathieu-Daudé]

On 10/10/23 11:58, Thomas Huth wrote:
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> > Additionally use qtest_get_arch_bits() when relevant.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   tests/qtest/ahci-test.c           |  5 ++---
> >   tests/qtest/am53c974-test.c       |  4 +---
> >   tests/qtest/bios-tables-test.c    |  6 +++---
> >   tests/qtest/boot-sector.c         |  6 +++---
> >   tests/qtest/device-plug-test.c    |  6 ++----
> >   tests/qtest/drive_del-test.c      | 16 ++++++----------


> > @@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
> >       }
> >       /* Check architecture */
> > -    arch = qtest_get_arch();
> > -    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> > +    if (strcmp(qtest_get_base_arch(), "x86")) {
> >           g_test_message("Skipping test for non-x86");
> >           return 0;
> >       }
>
> While this change makes sense (unifying two checks into one) ...
>
> > diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
> > index ed3ac7db20..dc41182a38 100644
> > --- a/tests/qtest/am53c974-test.c
> > +++ b/tests/qtest/am53c974-test.c
> > @@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
> >   int main(int argc, char **argv)
> >   {
> > -    const char *arch = qtest_get_arch();
> > -
> >       g_test_init(&argc, &argv, NULL);
> > -    if (strcmp(arch, "i386") == 0) {
> > +    if (!strcmp(qtest_get_base_arch(), "x86") && 
> > qtest_get_arch_bits() == 32) {
>
> ... this change looks more cumbersome now (doing two checks now instead 
> of one), at least at the current point in time. Do you urgently need 
> this for your refactoring? If not, I'd maybe postpone such changes that 
> make the checks more compilcated to a later point in time.

I wanted to replace qtest_get_arch() completely, but not that simple :(