Re: [PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Tue, 2023-04-25 at 09:19 +0200, Thomas Huth wrote:
> On 24/04/2023 15.10, Ilya Leoshkevich wrote:
> > On Mon, 2023-04-24 at 14:00 +0100, Alex Bennée wrote:
> > > 
> > > Ilya Leoshkevich <iii@linux.ibm.com> writes:
> > > 
> > > > The QEMU headers contain macros and functions that are useful
> > > > in
> > > > the
> > > > test context. Add them to tests' include path. Also provide a
> > > > header
> > > > similar to "qemu/osdep.h" for use in the freestanding
> > > > environment.
> > > > 
> > > > Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>,
> > > > which
> > > > does
> > > > not work without <stdint.h>. Make use of the new header in
> > > > these
> > > > tests
> > > > in order to fix this.
> > > > 
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > >   tests/include/qemu/testdep.h   | 14 ++++++++++++++
> > > >   tests/tcg/Makefile.target      |  4 ++--
> > > >   tests/tcg/aarch64/sve-ioctls.c |  1 +
> > > >   tests/tcg/aarch64/sysregs.c    |  1 +
> > > >   4 files changed, 18 insertions(+), 2 deletions(-)
> > > >   create mode 100644 tests/include/qemu/testdep.h
> > > > 
> > > > diff --git a/tests/include/qemu/testdep.h
> > > > b/tests/include/qemu/testdep.h
> > > > new file mode 100644
> > > > index 00000000000..ddf7c543bf4
> > > > --- /dev/null
> > > > +++ b/tests/include/qemu/testdep.h
> > > > @@ -0,0 +1,14 @@
> > > > +/*
> > > > + * Common dependencies for QEMU tests.
> > > > + *
> > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > + */
> > > > +#ifndef QEMU_TESTDEP_H
> > > > +#define QEMU_TESTDEP_H
> > > > +
> > > > +#include <stdint.h>
> > > > +#include "qemu/compiler.h"
> > > > +
> > > > +#define g_assert_not_reached __builtin_trap
> > > > +
> > > > +#endif
> > > > diff --git a/tests/tcg/Makefile.target
> > > > b/tests/tcg/Makefile.target
> > > > index 8318caf9247..5474395e693 100644
> > > > --- a/tests/tcg/Makefile.target
> > > > +++ b/tests/tcg/Makefile.target
> > > > @@ -85,8 +85,8 @@ TESTS=
> > > >   # additional tests which may re-use existing binaries
> > > >   EXTRA_TESTS=
> > > >   
> > > > -# Start with a blank slate, the build targets get to add stuff
> > > > first
> > > > -CFLAGS=
> > > > +# Start with the minimal build flags, the build targets will
> > > > extend them
> > > > +CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
> > > >   LDFLAGS=
> > > 
> > > Hmm I'm not so sure about this. The tests are deliberately
> > > minimal in
> > > terms of their dependencies because its hard enough getting a
> > > plain
> > > cross-compiler to work. Is there really much benefit to allowing
> > > this?
> > > What happens when a user includes another header which relies on
> > > functionality from one of the many libraries QEMU itself links
> > > to?
> > 
> > I don't think this will work at all, because the idea here is to
> > allow
> > using the code in the freestanding tests. However, at least bswap.h
> > seems to work just fine. Of course, there is now additional
> > maintenance
> > overhead to keep it this way, but I would argue it's better than
> > making a copy.
> 
> If this is just about one single header, I guess a
> 
> #include "../../../include/qemu/bswap.h"
> 
> would be acceptable, too, instead?
> 
>   Thomas

This would work, yes, but it would require essentially inlining
the new testdeps.h before it.