All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: "P. Berrange, Daniel" <berrange@redhat.com>,
	Sergio Lopez Pascual <slp@redhat.com>,
	"Hajnoczi, Stefan" <stefanha@gmail.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Armbruster, Markus" <armbru@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: Re: [PATCH] PoC: Rust binding for QAPI (qemu-ga only, for now)
Date: Fri, 11 Sep 2020 17:17:20 +0200	[thread overview]
Message-ID: <093067ee-e849-be8c-6e02-167d28d3e3ed@redhat.com> (raw)
In-Reply-To: <CAJ+F1CKPyBfjPoaRc5j24swo8Bfy0D7m+txNk5cyAi47rOz2bw@mail.gmail.com>

On 11/09/20 16:00, Marc-André Lureau wrote:
>     - from_qemu_none should be a "to_*" or "constructor" conversion (I used
>     new_from_foreign)
> 
> new_ prefix is not very rusty.

Right, I have changed it to with_foreign so now there is
{as,with,to,into}_foreign, plus unsafe_{from,into}.

These two could even be renamed to from_foreign and into_native at the
cost of making the trait less general purpose.  This way we have the
typical Rust names: as_* for Borrowed->Borrowed, with_*/to_* for
Borrowed->Owned, from_*/into_* for Owned->Owned.

> However, the memory allocator (or the stack) may not be compatible
> with the one used in C.

Hmm, that's a good point.  The simplest solution might be just to get
rid of IntoForeign, it's just an optimization.

> from_raw() is common, and takes ownership.

from_raw()/into_raw() would be equivalent to
into_foreign()/from_foreign().  However as you point out the allocators
are different, so it's a good idea IMHO to separate
into_raw()/from_raw() for the Rust allocator from
into_foreign()/from_foreign() for the libc allocator.

> I would need to modify this PoC for example

Yes of course.  Can you just try splitting the PoC in multiple patches?
 That should also make it easier to review, so far all I did was
comparing against glib-rs.

> But I must say I feel quite comfortable with the glib approach. It
> would be nice to have some feedback from glib-rs maintainers about your
> proposal.

QAPI is not tied to glib-rs, so I don't think qemu-ga will need to use
glib-rs.  I think either we use glib-rs, or if we are to roll our own we
should not be tied to the naming.  We don't use GObject introspection,
so none/full means nothing to most QEMU developers (and to Rust
developers too).

There are other things I don't like very much in glib-rs, for example
the use of tuples and public fields and the somewhat messy usage of
*const/*mut (I tried to be stricter on that).

Paolo



  reply	other threads:[~2020-09-11 15:19 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 17:48 [PATCH] PoC: Rust binding for QAPI (qemu-ga only, for now) marcandre.lureau
2020-09-11 10:24 ` Paolo Bonzini
2020-09-11 13:08   ` Paolo Bonzini
2020-09-11 14:00     ` Marc-André Lureau
2020-09-11 15:17       ` Paolo Bonzini [this message]
2020-09-29 17:55         ` Marc-André Lureau
2020-09-29 18:23           ` Paolo Bonzini
2020-09-30  9:15             ` Marc-André Lureau
2020-09-30 14:02               ` Paolo Bonzini
2020-09-11 10:46 ` Daniel P. Berrangé
2020-09-11 11:03   ` Marc-André Lureau
2020-09-11 11:28   ` Paolo Bonzini
2020-09-11 11:31     ` Daniel P. Berrangé
2020-09-11 14:19     ` John Snow
2020-09-11 14:17 ` Marc-André Lureau
2020-09-21  9:16 ` Markus Armbruster
2020-09-21  9:30   ` Paolo Bonzini
2020-09-22 14:59     ` Markus Armbruster
2020-09-21 10:04   ` Marc-André Lureau
2020-09-22 15:09     ` Markus Armbruster
2020-09-22 16:35       ` Marc-André Lureau
2020-09-22 17:08         ` Paolo Bonzini
2020-09-29  7:45           ` Marc-André Lureau
2020-09-29 10:14             ` Paolo Bonzini
2020-09-29 10:34               ` Marc-André Lureau
2020-09-29 11:00                 ` Paolo Bonzini
2020-09-29 11:34                   ` Marc-André Lureau
2020-09-30  7:34                     ` Markus Armbruster
2020-09-30  7:51                       ` Marc-André Lureau
2020-09-30 13:14                         ` Paolo Bonzini
2020-09-22 16:52       ` Daniel P. Berrangé
2020-09-23 11:51         ` Markus Armbruster
2020-09-21 10:11   ` Marc-André Lureau
2020-09-22 15:37     ` Markus Armbruster
2020-09-22 16:25       ` Marc-André Lureau
2020-09-24  7:31         ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=093067ee-e849-be8c-6e02-167d28d3e3ed@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=slp@redhat.com \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.