[PATCH 9/9] scripts/qapi: add required system includes to visitor

marcandre.lureau@redhat.com posted 9 patches 3 years, 7 months ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Markus Armbruster <armbru@redhat.com>, Laurent Vivier <laurent@vivier.eu>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Michael Roth <michael.roth@amd.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
[PATCH 9/9] scripts/qapi: add required system includes to visitor
Posted by marcandre.lureau@redhat.com 3 years, 7 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The generated visitor code includes abort() & assert(), we shouldn't
rely on the global "-i" headers to include the necessary system headers.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/visit.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 1ff464c0360f..d686df17f4b6 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -326,6 +326,8 @@ def __init__(self, prefix: str, include: List[str]):
     def _begin_builtin_module(self) -> None:
         self._genc.preamble_add(mcgen('''
 %(include)s
+#include <assert.h>
+#include <stdlib.h>
 
 #include "qapi/error.h"
 #include "qapi/qapi-builtin-visit.h"
@@ -342,6 +344,8 @@ def _begin_user_module(self, name: str) -> None:
         visit = self._module_basename('qapi-visit', name)
         self._genc.preamble_add(mcgen('''
 %(include)s
+#include <assert.h>
+#include <stdlib.h>
 
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
-- 
2.37.0.rc0


Re: [PATCH 9/9] scripts/qapi: add required system includes to visitor
Posted by Markus Armbruster 3 years, 7 months ago
marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The generated visitor code includes abort() & assert(), we shouldn't
> rely on the global "-i" headers to include the necessary system headers.

Suggest ", even though the default qemu/osdep.h always does.

>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/visit.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 1ff464c0360f..d686df17f4b6 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -326,6 +326,8 @@ def __init__(self, prefix: str, include: List[str]):
>      def _begin_builtin_module(self) -> None:
>          self._genc.preamble_add(mcgen('''
>  %(include)s
> +#include <assert.h>
> +#include <stdlib.h>
>  
>  #include "qapi/error.h"
>  #include "qapi/qapi-builtin-visit.h"
> @@ -342,6 +344,8 @@ def _begin_user_module(self, name: str) -> None:
>          visit = self._module_basename('qapi-visit', name)
>          self._genc.preamble_add(mcgen('''
>  %(include)s
> +#include <assert.h>
> +#include <stdlib.h>
>  
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"

Mildly irritating, because we normally kill such includes as redundant
on sight.

The builtin module (qapi-builtin-visit.c) doesn't actually need these
headers.  I guess you include them just in case that changes.
Re: [PATCH 9/9] scripts/qapi: add required system includes to visitor
Posted by Marc-André Lureau 3 years, 7 months ago
Hi

On Thu, Jun 23, 2022 at 5:43 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The generated visitor code includes abort() & assert(), we shouldn't
> > rely on the global "-i" headers to include the necessary system headers.
>
> Suggest ", even though the default qemu/osdep.h always does.
>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/qapi/visit.py | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> > index 1ff464c0360f..d686df17f4b6 100644
> > --- a/scripts/qapi/visit.py
> > +++ b/scripts/qapi/visit.py
> > @@ -326,6 +326,8 @@ def __init__(self, prefix: str, include: List[str]):
> >      def _begin_builtin_module(self) -> None:
> >          self._genc.preamble_add(mcgen('''
> >  %(include)s
> > +#include <assert.h>
> > +#include <stdlib.h>
> >
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-builtin-visit.h"
> > @@ -342,6 +344,8 @@ def _begin_user_module(self, name: str) -> None:
> >          visit = self._module_basename('qapi-visit', name)
> >          self._genc.preamble_add(mcgen('''
> >  %(include)s
> > +#include <assert.h>
> > +#include <stdlib.h>
> >
> >  #include "qapi/error.h"
> >  #include "qapi/qmp/qerror.h"
>
> Mildly irritating, because we normally kill such includes as redundant
> on sight.
>
> The builtin module (qapi-builtin-visit.c) doesn't actually need these
> headers.  I guess you include them just in case that changes.

True, at least not directly. I will drop it for now, we can add it
back when/if I figure out it is necessary.