[RFC v3 03/32] scripts/qapi: teach c_param_type() to return const argument type

marcandre.lureau@redhat.com posted 32 patches 4 years ago
[RFC v3 03/32] scripts/qapi: teach c_param_type() to return const argument type
Posted by marcandre.lureau@redhat.com 4 years ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The argument isn't owned by the callee, so it better be const.
But a lot of code in QEMU rely on non-const arguments to tweak it (steal
values etc).

Since Rust types / bindings are derived from the C version, we have to
be more accurate there to do correct ownership in the bindings.

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

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 3d72c7dfc9..1f6301c394 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -226,8 +226,15 @@ def c_type(self):
         pass
 
     # Return the C type to be used in a parameter list.
-    def c_param_type(self):
-        return self.c_type()
+    #
+    # The argument should be considered const, since no ownership is given to
+    # the callee, but qemu C code frequently tweaks it. Set const=True for a
+    # stricter declaration.
+    def c_param_type(self, const: bool = False):
+        c_type = self.c_type()
+        if const and c_type.endswith(POINTER_SUFFIX):
+            c_type = 'const ' + c_type
+        return c_type
 
     # Return the C type to be used where we suppress boxing.
     def c_unboxed_type(self):
@@ -280,10 +287,10 @@ def c_name(self):
     def c_type(self):
         return self._c_type_name
 
-    def c_param_type(self):
+    def c_param_type(self, const: bool = False):
         if self.name == 'str':
             return 'const ' + self._c_type_name
-        return self._c_type_name
+        return super().c_param_type(const)
 
     def json_type(self):
         return self._json_type_name
-- 
2.33.0.113.g6c40894d24


Re: [RFC v3 03/32] scripts/qapi: teach c_param_type() to return const argument type
Posted by Markus Armbruster 4 years ago
marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The argument isn't owned by the callee, so it better be const.
> But a lot of code in QEMU rely on non-const arguments to tweak it (steal
> values etc).
>
> Since Rust types / bindings are derived from the C version, we have to
> be more accurate there to do correct ownership in the bindings.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/schema.py | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 3d72c7dfc9..1f6301c394 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -226,8 +226,15 @@ def c_type(self):
>          pass
>  
>      # Return the C type to be used in a parameter list.
> -    def c_param_type(self):
> -        return self.c_type()
> +    #
> +    # The argument should be considered const, since no ownership is given to
> +    # the callee, but qemu C code frequently tweaks it. Set const=True for a
> +    # stricter declaration.

This comment makes sense only if you're familiar with Rust, where "may
change" is actually tied to ownership.

However, I can't see a use of .c_param_type(True).  Sure you need this
patch in this series?

> +    def c_param_type(self, const: bool = False):
> +        c_type = self.c_type()
> +        if const and c_type.endswith(POINTER_SUFFIX):
> +            c_type = 'const ' + c_type
> +        return c_type
>  
>      # Return the C type to be used where we suppress boxing.
>      def c_unboxed_type(self):
> @@ -280,10 +287,10 @@ def c_name(self):
>      def c_type(self):
>          return self._c_type_name
>  
> -    def c_param_type(self):
> +    def c_param_type(self, const: bool = False):
>          if self.name == 'str':
>              return 'const ' + self._c_type_name
> -        return self._c_type_name
> +        return super().c_param_type(const)

Would

       def c_param_type(self, const: bool = False):
           return super().c_param_type(const or self.name == 'str')

do?

>  
>      def json_type(self):
>          return self._json_type_name


Re: [RFC v3 03/32] scripts/qapi: teach c_param_type() to return const argument type
Posted by Marc-André Lureau 4 years ago
Hi

On Wed, Sep 8, 2021 at 4:12 PM Markus Armbruster <armbru@redhat.com> wrote:

> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The argument isn't owned by the callee, so it better be const.
> > But a lot of code in QEMU rely on non-const arguments to tweak it (steal
> > values etc).
> >
> > Since Rust types / bindings are derived from the C version, we have to
> > be more accurate there to do correct ownership in the bindings.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/qapi/schema.py | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index 3d72c7dfc9..1f6301c394 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -226,8 +226,15 @@ def c_type(self):
> >          pass
> >
> >      # Return the C type to be used in a parameter list.
> > -    def c_param_type(self):
> > -        return self.c_type()
> > +    #
> > +    # The argument should be considered const, since no ownership is
> given to
> > +    # the callee, but qemu C code frequently tweaks it. Set const=True
> for a
> > +    # stricter declaration.
>
> This comment makes sense only if you're familiar with Rust, where "may
> change" is actually tied to ownership.
>
>
Arguably, this semantic can also apply to C.


> However, I can't see a use of .c_param_type(True).  Sure you need this
> patch in this series?
>
>
Indeed it looks like a leftover now. Let's drop it.

> +    def c_param_type(self, const: bool = False):
> > +        c_type = self.c_type()
> > +        if const and c_type.endswith(POINTER_SUFFIX):
> > +            c_type = 'const ' + c_type
> > +        return c_type
> >
> >      # Return the C type to be used where we suppress boxing.
> >      def c_unboxed_type(self):
> > @@ -280,10 +287,10 @@ def c_name(self):
> >      def c_type(self):
> >          return self._c_type_name
> >
> > -    def c_param_type(self):
> > +    def c_param_type(self, const: bool = False):
> >          if self.name == 'str':
> >              return 'const ' + self._c_type_name
> > -        return self._c_type_name
> > +        return super().c_param_type(const)
>
> Would
>
>        def c_param_type(self, const: bool = False):
>            return super().c_param_type(const or self.name == 'str')
>
> do?
>
> >
> >      def json_type(self):
> >          return self._json_type_name
>
>
>

-- 
Marc-André Lureau