* [PATCH 00/14] rust: example of bindings code for Rust in QEMU
@ 2024-07-01 14:58 Paolo Bonzini
2024-07-01 14:58 ` [PATCH 01/14] add skeleton Paolo Bonzini
` (14 more replies)
0 siblings, 15 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Hi all,
this is an example of what some bindings code for QEMU would look like.
Note that some parts of this code are barely expected to compile, and
are probably full of bugs, but still should look like finished code
(in fact, because they compile, the type system parts should be okay;
though a conditional "should" is required).
This code is not integrated in the QEMU source tree, because again it
is just a example of what kind of Rust code would exist to handle the
C<->Rust FFI. The translation of a handful of C structs and function
prototypes is done by hand rather than with bindgen, in particular.
The patches are organized as follows:
Patches 1-2 introduce the skeleton for the rest of the code and are
not particularly interesting, since that skeleton would be provided
by the patches that introduce Rust usage in QEMU.
Patches 3-4 define common code to handle conversion of data structures
between Rust and C. I couldn't find an existing crate to do this,
though there are similar concepts in glib-rs. The crate defines
variants of Clone, From and Into that convert a Rust struct to a
C pointer, for example:
let s = "abc".clone_to_foreign(); // mallocs a NULL-terminated copy
let p = s.as_ptr();
drop(s); // p is freed now
or the other way round:
let s = String::cloned_from_foreign(p); // p is a *const c_char
let t: String = p.into_native(); // p is a *mut c_char and is free()d
The second patch defines what you need to convert strings from and to
C. It's used by tests but also by a couple of QOM functions implemented
below; it lets you forget the differences between String, &str, CString,
&CStr and Cow<'_, str>.
This is the only complete part of the skeleton (in fact I wrote it
a couple years ago), and it comes with testcases that pass in both
"#[test]" and "doctest" (i.e. snippets integrated in the documentation
comments) styles.
Patch 5 (mostly complete except for &error_abort support and missing
tests) allows using the above functionality for the QEMU Error*.
Patches 6-8 provide an example of how to use QOM from Rust. They
define all the functionality to:
- handle reference counting
- handle typesafe casting (i.e. code doesn't compile if an upcast
is invalid, or if a downcast cannot possibly succeed).
- faking inheritance since Rust doesn't support it.
While I started with some implementation ideas in glib-rs, this has
evolved quite a bit and doesn't really look much like glib-rs anymore.
I provided a handful of functions to wrap C APIs. For example:
object_property_add_child(obj, "foo", object_new(TYPE_BLAH))
becomes
obj.property_add_child("foo", Blah::new());
... except that the former leaks a reference and the latter does not
(gotcha :)). The idea is that these functions would be written, in
general, as soon as Rust code uses them, but I included a few as an
example of using the various abstractions for typecasting, reference
counting, and converting from/to C data strcutures.
A large part of this code is of the "it compiles and it looks nice, it
must be perfect" kind. Rust is _very_ picky on type safety, in case you
didn't know. One day we're all going to be programming in Haskell,
without even noticing it.
Patches 9-10 deal with how to define new subclasses in Rust. They are
a lot less polished and less ready. There is probably a lot of polish
that could be applied to make the code look nicer, but I guess there is
always time to make the code look nicer until the details are sorted out.
The things that I considered here are:
- splitting configuration from runtime state. Configuration is
immutable throughout the lifetime of the object, and holds the
value of user-configured properties. State will typically be a
Mutex<> or RefCell<> since the QOM bindings make wide use of interior
mutability---almost all functions are declared as &self---following
the lead of glib-rs.
- automatic generation of instance_init and instance_finalize. For
this I opted to introduce a new initialization step that is tailored to
Rust, called instance_mem_init(), that is executed before the default
value of properties is set. This makes sure that user code only ever
sees valid values for the whole struct, including after a downcast;
it matters because some Rust types (notably references) cannot be
initialized to a zero-bytes pattern. The default implementation of
instance_mem_init is simply a memset(), since the callback replaces the
memset(obj, 0, type->instance_size);
line in object_initialize_with_type(). I have prototyped this change
in QEMU already.
- generation of C vtables from safe code that is written in Rust. I
chose a trait that only contains associated constants as a way to
access the vtables generically. For example:
impl ObjectImpl for TestDevice {
const UNPARENT: Option<fn(&TestDevice)> = Some(TestDevice::unparent);
}
impl DeviceImpl for TestDevice {
const REALIZE: Option<fn(&TestDevice) -> Result<()>> = Some(TestDevice::realize);
}
This works and it seems like a style that (in the future) we could apply
macro or even procedural macro magic to.
- generation of qdev property tables. While only boolean properties are
implemented here, one idea that I experimented with, is that the
default value of properties is derived from the ConstDefault trait.
(ConstDefault is provided by the const_default external crate). Again,
this is material for future conversion to procedural macros.
I absolutely didn't look at vmstate, but it shouldn't be too different
from properties, at least for the common cases.
Patches 11-14 finally are an example of the changes that are needed
to respect a minimum supported Rust version consistent with what is in
Debian Bullseye. It's not too bad, especially since the current version
of the QOM bindings does not require generic associated types anymore.
Why am I posting this? Because this kind of glue code is the ultimate
source of technical debt. It is the thing that we should be scared of
when introducing a new language in QEMU. It makes it harder to change C
code, and it is hard to change once Rust code becomes more widespread.
If we think a C API is not fully baked, we probably shouldn't write
Rust code that uses it (including bindings code). If we think a Rust
API is not fully baked, we probably shouldn't add too much Rust code
that uses it.
We should have an idea of what this glue code looks like, in order to make
an informed choice. If we think we're not comfortable with reviewing it,
well, we should be ready to say so and stick with C until we are.
The alternative could be to use Rust without this kind of binding. I
think it's a bad idea. It removes many of the advantages of Rust
(which are exemplified by the above object_property_add_child one-liner),
and it also introduces _new_ kinds of memory errors, since Rust has
its own undefined behavior conditions that are not there in C/C++.
For example:
impl Struct {
pub fn f(&self) {
call_some_c_function(Self::g, self as *const Self as *mut _);
}
fn do_g(&mut self) {
...
}
extern "C" fn g(ptr: *mut Self) {
unsafe { &mut *ptr }.do_g();
}
}
is invalid because a &mut reference (exclusive) is alive at the same time
as a & reference (shared). It is left as an exercise to the reader to
figure out all the possible ways in which we can shoot our own feet,
considering the pervasive use of callbacks in QEMU.
With respect to callbacks, that's something that is missing in this
prototype. Fortunately, that's also something that will be tackled very
soon if the PL011 example is merged, because memory regions and character
devices both introduce them. Also, as I understand it, Rust code using
callbacks is not particularly nice anyway, though it is of course doable.
Instead, this exercise are about being able to write *nice* Rust code,
with all the advantages provided by the language, and the cost of
writing/maintaining the glue code that makes it possible. I expect
that we'll use a technique similar to the extern_c crate (it's 20 lines of
code; https://docs.rs/crate/extern-c/) to convert something that implements
Fn(), including a member function, into an extern "C" function.
Anyhow: I think we can do it, otherwise I would not have written 2000 lines
of code (some of it two or three times). But if people are now scared and
think we shouldn't, well, that's also a success of its own kind.
Paolo
Paolo Bonzini (14):
add skeleton
set expectations
rust: define traits and pointer wrappers to convert from/to C
representations
rust: add tests for util::foreign
rust: define wrappers for Error
rust: define wrappers for basic QOM concepts
rust: define wrappers for methods of the QOM Object class
rust: define wrappers for methods of the QOM Device class
rust: add idiomatic bindings to define Object subclasses
rust: add idiomatic bindings to define Device subclasses
rust: replace std::ffi::c_char with libc::c_char
rust: replace c"" literals with cstr crate
rust: introduce alternative to offset_of!
rust: use version of toml_edit that does not require new Rust
--
2.45.2
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 01/14] add skeleton
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 02/14] set expectations Paolo Bonzini
` (13 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
qemu/ is where target-independent code goes. This code should
not use constructors and will be brought in as needed.
qemu-hw/ is where the target-dependent code goes, which is going
to be built depending on Kconfig symbols.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
.gitignore | 2 +
Cargo.toml | 3 +
qemu-hw/Cargo.toml | 6 ++
qemu-hw/src/lib.rs | 0
qemu-hw/src/main.rs | 3 +
qemu/Cargo.toml | 7 ++
qemu/src/bindings/mod.rs | 88 +++++++++++++++++++++++++
qemu/src/lib.rs | 4 ++
9 files changed, 249 insertions(+)
create mode 100644 .gitignore
create mode 100644 Cargo.toml
create mode 100644 qemu-hw/Cargo.toml
create mode 100644 qemu-hw/src/lib.rs
create mode 100644 qemu-hw/src/main.rs
create mode 100644 qemu/Cargo.toml
create mode 100644 qemu/src/bindings/mod.rs
create mode 100644 qemu/src/lib.rs
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..3fbfc34
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+/target
+/.git-old
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 0000000..f66a80e
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,3 @@
+[workspace]
+members = ["qemu"]
+resolver = "2"
diff --git a/qemu-hw/Cargo.toml b/qemu-hw/Cargo.toml
new file mode 100644
index 0000000..9bd6930
--- /dev/null
+++ b/qemu-hw/Cargo.toml
@@ -0,0 +1,6 @@
+[package]
+name = "qemu-hw"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
diff --git a/qemu-hw/src/lib.rs b/qemu-hw/src/lib.rs
new file mode 100644
index 0000000..e69de29
diff --git a/qemu-hw/src/main.rs b/qemu-hw/src/main.rs
new file mode 100644
index 0000000..e7a11a9
--- /dev/null
+++ b/qemu-hw/src/main.rs
@@ -0,0 +1,3 @@
+fn main() {
+ println!("Hello, world!");
+}
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
new file mode 100644
index 0000000..18d0fa4
--- /dev/null
+++ b/qemu/Cargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "qemu"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+const-default = { version = "~1", features = ["derive"] }
diff --git a/qemu/src/bindings/mod.rs b/qemu/src/bindings/mod.rs
new file mode 100644
index 0000000..a49447b
--- /dev/null
+++ b/qemu/src/bindings/mod.rs
@@ -0,0 +1,88 @@
+use std::ffi::{c_char, c_void};
+
+#[repr(C)]
+pub struct Object {
+ pub klass: *mut c_void,
+ pub free: extern "C" fn(c: *mut c_void),
+ pub properties: *mut c_void,
+ pub r#ref: u32,
+ pub parent: *mut Object,
+}
+
+#[repr(C)]
+pub struct ObjectClass {
+ pub unparent: Option<unsafe extern "C" fn(*mut Object)>,
+}
+
+#[repr(C)]
+pub struct DeviceState {
+ pub base: Object,
+}
+
+#[repr(C)]
+#[allow(non_camel_case_types)]
+pub struct PropertyInfo {
+ pub name: *const c_char,
+ pub description: *const c_char,
+ // ...
+}
+#[repr(C)]
+pub struct Property {
+ pub name: *const c_char,
+ pub offset: usize,
+ pub default: u64,
+ pub info: *const PropertyInfo,
+}
+
+pub struct DeviceClass {
+ pub oc: ObjectClass,
+
+ pub realize: Option<unsafe extern "C" fn(*mut DeviceState, *mut *mut Error)>,
+ pub unrealize: Option<unsafe extern "C" fn(*mut DeviceState)>,
+ pub cold_reset: Option<unsafe extern "C" fn(*mut DeviceState)>,
+ pub properties: *const Property,
+}
+
+#[repr(C)]
+pub struct TypeInfo {
+ pub name: *const c_char,
+ pub parent: *const c_char,
+ pub instance_mem_init: Option<unsafe extern "C" fn(*mut c_void)>,
+ pub instance_init: Option<unsafe extern "C" fn(*mut c_void)>,
+ pub instance_finalize: Option<unsafe extern "C" fn(*mut c_void)>,
+ pub class_init: Option<unsafe extern "C" fn(*mut c_void, *mut c_void)>,
+ pub instance_size: usize,
+}
+
+#[repr(C)]
+pub struct Error {
+ _unused: c_char,
+}
+
+extern "C" {
+ pub fn error_setg_internal(
+ errp: *mut *mut Error,
+ src: *mut c_char,
+ line: u32,
+ func: *mut c_char,
+ fmt: *const c_char,
+ ...
+ );
+ pub fn error_get_pretty(errp: *const Error) -> *mut c_char;
+ pub fn error_free(errp: *mut Error);
+
+ pub fn object_dynamic_cast(obj: *mut Object, typ: *const c_char) -> *mut c_void;
+ pub fn object_property_add_child(obj: *mut Object, typ: *const c_char,
+ child: *mut Object);
+ pub fn object_get_typename(obj: *const Object) -> *const c_char;
+ pub fn object_ref(obj: *mut Object);
+ pub fn object_new(typ: *const c_char) -> *const Object;
+ pub fn object_unref(obj: *mut Object);
+ pub fn object_unparent(obj: *mut Object);
+
+ pub fn device_cold_reset(obj: *mut DeviceState);
+ pub fn device_realize(obj: *mut DeviceState, err: *mut *mut Error) -> bool;
+ pub fn type_register(obj: *const TypeInfo);
+
+ pub static qdev_prop_bool: PropertyInfo;
+}
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
new file mode 100644
index 0000000..fce21d7
--- /dev/null
+++ b/qemu/src/lib.rs
@@ -0,0 +1,4 @@
+#![allow(unused_macros)]
+#![allow(dead_code)]
+
+pub mod bindings;
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 02/14] set expectations
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
2024-07-01 14:58 ` [PATCH 01/14] add skeleton Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
` (12 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
README.md | 1 +
1 file changed, 1 insertion(+)
create mode 100644 README.md
diff --git a/README.md b/README.md
new file mode 100644
index 0000000..5ef6f0d
--- /dev/null
+++ b/README.md
@@ -0,0 +1 @@
+This is very experimental and barely compiles
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
2024-07-01 14:58 ` [PATCH 01/14] add skeleton Paolo Bonzini
2024-07-01 14:58 ` [PATCH 02/14] set expectations Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-03 12:48 ` Marc-André Lureau
` (2 more replies)
2024-07-01 14:58 ` [PATCH 04/14] rust: add tests for util::foreign Paolo Bonzini
` (11 subsequent siblings)
14 siblings, 3 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
The qemu::util::foreign module provides:
- A trait for structs that can be converted to a C ("foreign") representation
(CloneToForeign)
- A trait for structs that can be built from a C ("foreign") representation
(FromForeign), and the utility IntoNative that can be used with less typing
(similar to the standard library's From and Into pair)
- Automatic implementations of the above traits for Option<>, supporting NULL
pointers
- A wrapper for a pointer that automatically frees the contained data. If
a struct XYZ implements CloneToForeign, you can build an OwnedPointer<XYZ>
and it will free the contents automatically unless you retrieve it with
owned_ptr.into_inner()
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/lib.rs | 6 +
qemu/src/util/foreign.rs | 247 +++++++++++++++++++++++++++++++++++++++
qemu/src/util/mod.rs | 1 +
3 files changed, 254 insertions(+)
create mode 100644 qemu/src/util/foreign.rs
create mode 100644 qemu/src/util/mod.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index fce21d7..c48edcf 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,3 +2,9 @@
#![allow(dead_code)]
pub mod bindings;
+
+pub mod util;
+pub use util::foreign::CloneToForeign;
+pub use util::foreign::FromForeign;
+pub use util::foreign::IntoNative;
+pub use util::foreign::OwnedPointer;
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
new file mode 100644
index 0000000..a591925
--- /dev/null
+++ b/qemu/src/util/foreign.rs
@@ -0,0 +1,247 @@
+// TODO: change to use .cast() etc.
+#![allow(clippy::ptr_as_ptr)]
+
+/// Traits to map between C structs and native Rust types.
+/// Similar to glib-rs but a bit simpler and possibly more
+/// idiomatic.
+use std::borrow::Cow;
+use std::fmt;
+use std::fmt::Debug;
+use std::mem;
+use std::ptr;
+
+/// A type for which there is a canonical representation as a C datum.
+pub trait CloneToForeign {
+ /// The representation of `Self` as a C datum. Typically a
+ /// `struct`, though there are exceptions for example `c_char`
+ /// for strings, since C strings are of `char *` type).
+ type Foreign;
+
+ /// Free the C datum pointed to by `p`.
+ ///
+ /// # Safety
+ ///
+ /// `p` must be `NULL` or point to valid data.
+ unsafe fn free_foreign(p: *mut Self::Foreign);
+
+ /// Convert a native Rust object to a foreign C struct, copying
+ /// everything pointed to by `self` (same as `to_glib_full` in `glib-rs`)
+ fn clone_to_foreign(&self) -> OwnedPointer<Self>;
+
+ /// Convert a native Rust object to a foreign C pointer, copying
+ /// everything pointed to by `self`. The returned pointer must
+ /// be freed with the `free_foreign` associated function.
+ fn clone_to_foreign_ptr(&self) -> *mut Self::Foreign {
+ self.clone_to_foreign().into_inner()
+ }
+}
+
+impl<T> CloneToForeign for Option<T>
+where
+ T: CloneToForeign,
+{
+ type Foreign = <T as CloneToForeign>::Foreign;
+
+ unsafe fn free_foreign(x: *mut Self::Foreign) {
+ T::free_foreign(x)
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // Same as the underlying implementation, but also convert `None`
+ // to a `NULL` pointer.
+ self.as_ref()
+ .map(CloneToForeign::clone_to_foreign)
+ .map(OwnedPointer::into)
+ .unwrap_or_default()
+ }
+}
+
+impl<T> FromForeign for Option<T>
+where
+ T: FromForeign,
+{
+ unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
+ // Same as the underlying implementation, but also accept a `NULL` pointer.
+ if p.is_null() {
+ None
+ } else {
+ Some(T::cloned_from_foreign(p))
+ }
+ }
+}
+
+impl<T> CloneToForeign for Box<T>
+where
+ T: CloneToForeign,
+{
+ type Foreign = <T as CloneToForeign>::Foreign;
+
+ unsafe fn free_foreign(x: *mut Self::Foreign) {
+ T::free_foreign(x)
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ self.as_ref().clone_to_foreign().into()
+ }
+}
+
+impl<T> FromForeign for Box<T>
+where
+ T: FromForeign,
+{
+ unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
+ Box::new(T::cloned_from_foreign(p))
+ }
+}
+
+impl<'a, B> CloneToForeign for Cow<'a, B>
+ where B: 'a + ToOwned + ?Sized + CloneToForeign,
+{
+ type Foreign = B::Foreign;
+
+ unsafe fn free_foreign(ptr: *mut B::Foreign) {
+ B::free_foreign(ptr);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ self.as_ref().clone_to_foreign().into()
+ }
+}
+
+/// Convert a C datum into a native Rust object, taking ownership of
+/// the C datum. You should not need to implement this trait
+/// as long as Rust types implement `FromForeign`.
+pub trait IntoNative<T> {
+ /// Convert a C datum to a native Rust object, taking ownership of
+ /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
+ ///
+ /// # Safety
+ ///
+ /// `p` must point to valid data, or can be `NULL` if Self is an
+ /// `Option` type. It becomes invalid after the function returns.
+ unsafe fn into_native(self) -> T;
+}
+
+impl<T, U> IntoNative<U> for *mut T
+where
+ U: FromForeign<Foreign = T>,
+{
+ unsafe fn into_native(self) -> U {
+ U::from_foreign(self)
+ }
+}
+
+/// A type which can be constructed from a canonical representation as a
+/// C datum.
+pub trait FromForeign: CloneToForeign + Sized {
+ /// Convert a C datum to a native Rust object, copying everything
+ /// pointed to by `p` (same as `from_glib_none` in `glib-rs`)
+ ///
+ /// # Safety
+ ///
+ /// `p` must point to valid data, or can be `NULL` is `Self` is an
+ /// `Option` type.
+ unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self;
+
+ /// Convert a C datum to a native Rust object, taking ownership of
+ /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
+ ///
+ /// The default implementation calls `cloned_from_foreign` and frees `p`.
+ ///
+ /// # Safety
+ ///
+ /// `p` must point to valid data, or can be `NULL` is `Self` is an
+ /// `Option` type. `p` becomes invalid after the function returns.
+ unsafe fn from_foreign(p: *mut Self::Foreign) -> Self {
+ let result = Self::cloned_from_foreign(p);
+ Self::free_foreign(p);
+ result
+ }
+}
+
+pub struct OwnedPointer<T: CloneToForeign + ?Sized> {
+ ptr: *mut <T as CloneToForeign>::Foreign,
+}
+
+impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
+ /// Return a new `OwnedPointer` that wraps the pointer `ptr`.
+ ///
+ /// # Safety
+ ///
+ /// The pointer must be valid and live until the returned `OwnedPointer`
+ /// is dropped.
+ pub unsafe fn new(ptr: *mut <T as CloneToForeign>::Foreign) -> Self {
+ OwnedPointer { ptr }
+ }
+
+ /// Safely create an `OwnedPointer` from one that has the same
+ /// freeing function.
+ pub fn from<U>(x: OwnedPointer<U>) -> Self
+ where
+ U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign> + ?Sized,
+ {
+ unsafe {
+ // SAFETY: the pointer type and free function are the same,
+ // only the type changes
+ OwnedPointer::new(x.into_inner())
+ }
+ }
+
+ /// Safely convert an `OwnedPointer` into one that has the same
+ /// freeing function.
+ pub fn into<U>(self) -> OwnedPointer<U>
+ where
+ U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign>,
+ {
+ OwnedPointer::from(self)
+ }
+
+ /// Return the pointer that is stored in the `OwnedPointer`. The
+ /// pointer is valid for as long as the `OwnedPointer` itself.
+ pub fn as_ptr(&self) -> *const <T as CloneToForeign>::Foreign {
+ self.ptr
+ }
+
+ pub fn as_mut_ptr(&self) -> *mut <T as CloneToForeign>::Foreign {
+ self.ptr
+ }
+
+ /// Return the pointer that is stored in the `OwnedPointer`,
+ /// consuming the `OwnedPointer` but not freeing the pointer.
+ pub fn into_inner(mut self) -> *mut <T as CloneToForeign>::Foreign {
+ let result = mem::replace(&mut self.ptr, ptr::null_mut());
+ mem::forget(self);
+ result
+ }
+}
+
+impl<T: FromForeign + ?Sized> OwnedPointer<T> {
+ /// Convert a C datum to a native Rust object, taking ownership of
+ /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
+ pub fn into_native(self) -> T {
+ // SAFETY: the pointer was passed to the unsafe constructor OwnedPointer::new
+ unsafe { T::from_foreign(self.into_inner()) }
+ }
+}
+
+impl<T: CloneToForeign + ?Sized> Debug for OwnedPointer<T> {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let name = std::any::type_name::<T>();
+ let name = format!("OwnedPointer<{}>", name);
+ f.debug_tuple(&name).field(&self.as_ptr()).finish()
+ }
+}
+
+impl<T: CloneToForeign + Default + ?Sized> Default for OwnedPointer<T> {
+ fn default() -> Self {
+ <T as Default>::default().clone_to_foreign()
+ }
+}
+
+impl<T: CloneToForeign + ?Sized> Drop for OwnedPointer<T> {
+ fn drop(&mut self) {
+ let p = mem::replace(&mut self.ptr, ptr::null_mut());
+ // SAFETY: the pointer was passed to the unsafe constructor OwnedPointer::new
+ unsafe { T::free_foreign(p) }
+ }
+}
diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
new file mode 100644
index 0000000..be00d0c
--- /dev/null
+++ b/qemu/src/util/mod.rs
@@ -0,0 +1 @@
+pub mod foreign;
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 04/14] rust: add tests for util::foreign
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (2 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 05/14] rust: define wrappers for Error Paolo Bonzini
` (10 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Provide sample implementations in util::foreign for strings and
elementary integer types, and use them to test the code.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/Cargo.toml | 4 +
qemu/src/util/foreign.rs | 456 +++++++++++++++++++++++++++++++++++++++
3 files changed, 474 insertions(+)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 18d0fa4..1100725 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -5,3 +5,7 @@ edition = "2021"
[dependencies]
const-default = { version = "~1", features = ["derive"] }
+libc = "^0"
+
+[dev-dependencies]
+matches = ">=0"
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index a591925..0b8b708 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -5,6 +5,7 @@
/// Similar to glib-rs but a bit simpler and possibly more
/// idiomatic.
use std::borrow::Cow;
+use std::ffi::{c_char, c_void, CStr, CString};
use std::fmt;
use std::fmt::Debug;
use std::mem;
@@ -22,6 +23,14 @@ pub trait CloneToForeign {
/// # Safety
///
/// `p` must be `NULL` or point to valid data.
+ ///
+ /// ```
+ /// # use qemu::CloneToForeign;
+ /// let foreign = "Hello, world!".clone_to_foreign();
+ /// unsafe {
+ /// String::free_foreign(foreign.into_inner());
+ /// }
+ /// ```
unsafe fn free_foreign(p: *mut Self::Foreign);
/// Convert a native Rust object to a foreign C struct, copying
@@ -119,6 +128,17 @@ pub trait IntoNative<T> {
///
/// `p` must point to valid data, or can be `NULL` if Self is an
/// `Option` type. It becomes invalid after the function returns.
+ ///
+ /// ```
+ /// # use qemu::{CloneToForeign, IntoNative};
+ /// let s = "Hello, world!".to_string();
+ /// let foreign = s.clone_to_foreign();
+ /// let native: String = unsafe {
+ /// foreign.into_native()
+ /// // foreign is not leaked
+ /// };
+ /// assert_eq!(s, native);
+ /// ```
unsafe fn into_native(self) -> T;
}
@@ -141,6 +161,15 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// `p` must point to valid data, or can be `NULL` is `Self` is an
/// `Option` type.
+ ///
+ /// ```
+ /// # use qemu::FromForeign;
+ /// let p = c"Hello, world!".as_ptr();
+ /// let s = unsafe {
+ /// String::cloned_from_foreign(p as *const std::ffi::c_char)
+ /// };
+ /// assert_eq!(s, "Hello, world!");
+ /// ```
unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self;
/// Convert a C datum to a native Rust object, taking ownership of
@@ -152,6 +181,16 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// `p` must point to valid data, or can be `NULL` is `Self` is an
/// `Option` type. `p` becomes invalid after the function returns.
+ ///
+ /// ```
+ /// # use qemu::{CloneToForeign, FromForeign};
+ /// let s = "Hello, world!";
+ /// let foreign = s.clone_to_foreign();
+ /// unsafe {
+ /// assert_eq!(String::from_foreign(foreign.into_inner()), s);
+ /// }
+ /// // foreign is not leaked
+ /// ```
unsafe fn from_foreign(p: *mut Self::Foreign) -> Self {
let result = Self::cloned_from_foreign(p);
Self::free_foreign(p);
@@ -176,6 +215,12 @@ impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
/// Safely create an `OwnedPointer` from one that has the same
/// freeing function.
+ /// ```
+ /// # use qemu::{CloneToForeign, OwnedPointer};
+ /// let s = "Hello, world!";
+ /// let foreign_str = s.clone_to_foreign();
+ /// let foreign_string = OwnedPointer::<String>::from(foreign_str);
+ /// # assert_eq!(foreign_string.into_native(), s);
pub fn from<U>(x: OwnedPointer<U>) -> Self
where
U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign> + ?Sized,
@@ -189,6 +234,12 @@ impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
/// Safely convert an `OwnedPointer` into one that has the same
/// freeing function.
+ /// ```
+ /// # use qemu::{CloneToForeign, OwnedPointer};
+ /// let s = "Hello, world!";
+ /// let foreign_str = s.clone_to_foreign();
+ /// let foreign_string: OwnedPointer<String> = foreign_str.into();
+ /// # assert_eq!(foreign_string.into_native(), s);
pub fn into<U>(self) -> OwnedPointer<U>
where
U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign>,
@@ -198,6 +249,16 @@ impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
/// Return the pointer that is stored in the `OwnedPointer`. The
/// pointer is valid for as long as the `OwnedPointer` itself.
+ ///
+ /// ```
+ /// # use qemu::CloneToForeign;
+ /// let s = "Hello, world!";
+ /// let foreign = s.clone_to_foreign();
+ /// let p = foreign.as_ptr();
+ /// let len = unsafe { libc::strlen(p) };
+ /// drop(foreign);
+ /// # assert_eq!(len, 13);
+ /// ```
pub fn as_ptr(&self) -> *const <T as CloneToForeign>::Foreign {
self.ptr
}
@@ -208,6 +269,15 @@ impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
/// Return the pointer that is stored in the `OwnedPointer`,
/// consuming the `OwnedPointer` but not freeing the pointer.
+ ///
+ /// ```
+ /// # use qemu::CloneToForeign;
+ /// let s = "Hello, world!";
+ /// let p = s.clone_to_foreign().into_inner();
+ /// let len = unsafe { libc::strlen(p) };
+ /// // p needs to be freed manually
+ /// # assert_eq!(len, 13);
+ /// ```
pub fn into_inner(mut self) -> *mut <T as CloneToForeign>::Foreign {
let result = mem::replace(&mut self.ptr, ptr::null_mut());
mem::forget(self);
@@ -218,6 +288,17 @@ impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
impl<T: FromForeign + ?Sized> OwnedPointer<T> {
/// Convert a C datum to a native Rust object, taking ownership of
/// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
+ ///
+ /// ```
+ /// # use qemu::{CloneToForeign, IntoNative};
+ /// let s = "Hello, world!".to_string();
+ /// let foreign = s.clone_to_foreign();
+ /// let native: String = unsafe {
+ /// foreign.into_native()
+ /// // foreign is not leaked
+ /// };
+ /// assert_eq!(s, native);
+ /// ```
pub fn into_native(self) -> T {
// SAFETY: the pointer was passed to the unsafe constructor OwnedPointer::new
unsafe { T::from_foreign(self.into_inner()) }
@@ -245,3 +326,378 @@ impl<T: CloneToForeign + ?Sized> Drop for OwnedPointer<T> {
unsafe { T::free_foreign(p) }
}
}
+
+impl CloneToForeign for str {
+ type Foreign = c_char;
+
+ unsafe fn free_foreign(ptr: *mut c_char) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // SAFETY: self.as_ptr() is guaranteed to point to self.len() bytes;
+ // the destination is freshly allocated
+ unsafe {
+ let p = libc::malloc(self.len() + 1) as *mut c_char;
+ ptr::copy_nonoverlapping(self.as_ptr() as *const c_char, p, self.len());
+ *p.add(self.len()) = 0;
+ OwnedPointer::new(p)
+ }
+ }
+}
+
+impl CloneToForeign for CStr {
+ type Foreign = c_char;
+
+ unsafe fn free_foreign(ptr: *mut c_char) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // SAFETY: self.as_ptr() is guaranteed to point to self.len() bytes;
+ // the destination is freshly allocated
+ unsafe {
+ let slice = self.to_bytes_with_nul();
+ let p = libc::malloc(slice.len()) as *mut c_char;
+ ptr::copy_nonoverlapping(self.as_ptr() as *const c_char, p, slice.len());
+ OwnedPointer::new(p)
+ }
+ }
+}
+
+impl CloneToForeign for String {
+ type Foreign = c_char;
+
+ unsafe fn free_foreign(ptr: *mut c_char) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ self.as_str().clone_to_foreign().into()
+ }
+}
+
+impl FromForeign for String {
+ unsafe fn cloned_from_foreign(p: *const c_char) -> Self {
+ let cstr = CStr::from_ptr(p);
+ String::from_utf8_lossy(cstr.to_bytes()).into_owned()
+ }
+}
+
+impl CloneToForeign for CString {
+ type Foreign = c_char;
+
+ unsafe fn free_foreign(ptr: *mut c_char) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ self.as_c_str().clone_to_foreign().into()
+ }
+}
+
+impl FromForeign for CString {
+ unsafe fn cloned_from_foreign(p: *const c_char) -> Self {
+ CStr::from_ptr(p).to_owned()
+ }
+}
+
+impl FromForeign for Cow<'_, str>
+{
+ unsafe fn cloned_from_foreign(p: *const c_char) -> Self {
+ let cstr = CStr::from_ptr(p);
+ cstr.to_string_lossy()
+ }
+}
+
+macro_rules! foreign_copy_type {
+ ($rust_type:ty, $foreign_type:ty) => {
+ impl CloneToForeign for $rust_type {
+ type Foreign = $foreign_type;
+
+ unsafe fn free_foreign(ptr: *mut Self::Foreign) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // Safety: we are copying into a freshly-allocated block
+ unsafe {
+ let p = libc::malloc(mem::size_of::<Self>()) as *mut Self::Foreign;
+ *p = *self as Self::Foreign;
+ OwnedPointer::new(p)
+ }
+ }
+ }
+
+ impl FromForeign for $rust_type {
+ unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
+ *p
+ }
+ }
+
+ impl CloneToForeign for [$rust_type] {
+ type Foreign = $foreign_type;
+
+ unsafe fn free_foreign(ptr: *mut Self::Foreign) {
+ libc::free(ptr as *mut c_void);
+ }
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // SAFETY: self.as_ptr() is guaranteed to point to the same number of bytes
+ // as the freshly allocated destination
+ unsafe {
+ let size = mem::size_of::<Self::Foreign>();
+ let p = libc::malloc(self.len() * size) as *mut Self::Foreign;
+ ptr::copy_nonoverlapping(self.as_ptr() as *const Self::Foreign, p, self.len());
+ OwnedPointer::new(p)
+ }
+ }
+ }
+ };
+}
+foreign_copy_type!(i8, i8);
+foreign_copy_type!(u8, u8);
+foreign_copy_type!(i16, i16);
+foreign_copy_type!(u16, u16);
+foreign_copy_type!(i32, i32);
+foreign_copy_type!(u32, u32);
+foreign_copy_type!(i64, i64);
+foreign_copy_type!(u64, u64);
+foreign_copy_type!(isize, libc::ptrdiff_t);
+foreign_copy_type!(usize, libc::size_t);
+foreign_copy_type!(f32, f32);
+foreign_copy_type!(f64, f64);
+
+#[cfg(test)]
+mod tests {
+ #![allow(clippy::shadow_unrelated)]
+
+ use super::*;
+ use matches::assert_matches;
+ use std::ffi::c_void;
+
+ #[test]
+ fn test_foreign_int_convert() {
+ let i = 123i8;
+ let p = i.clone_to_foreign();
+ unsafe {
+ assert_eq!(i, *p.as_ptr());
+ assert_eq!(i, i8::cloned_from_foreign(p.as_ptr()));
+ }
+ assert_eq!(i, p.into_native());
+
+ let p = i.clone_to_foreign();
+ unsafe {
+ assert_eq!(i, i8::from_foreign(p.into_inner()));
+ }
+ }
+
+ #[test]
+ fn test_cloned_from_foreign_string_cow() {
+ let s = "Hello, world!".to_string();
+ let cstr = c"Hello, world!";
+ let cloned = unsafe { Cow::cloned_from_foreign(cstr.as_ptr()) };
+ assert_eq!(s, cloned);
+ }
+
+ #[test]
+ fn test_cloned_from_foreign_string() {
+ let s = "Hello, world!".to_string();
+ let cstr = c"Hello, world!";
+ let cloned = unsafe { String::cloned_from_foreign(cstr.as_ptr()) };
+ assert_eq!(s, cloned);
+ }
+
+ #[test]
+ fn test_cloned_from_foreign_cstring() {
+ let s = CString::new("Hello, world!").unwrap();
+ let cloned = s.clone_to_foreign();
+ let copy = unsafe { CString::cloned_from_foreign(cloned.as_ptr()) };
+ assert_ne!(copy.as_ptr(), cloned.as_ptr());
+ assert_ne!(copy.as_ptr(), s.as_ptr());
+ assert_eq!(copy, s);
+ }
+
+ #[test]
+ fn test_from_foreign_string() {
+ let s = "Hello, world!".to_string();
+ let cloned = s.clone_to_foreign_ptr();
+ let copy = unsafe { String::from_foreign(cloned) };
+ assert_eq!(s, copy);
+ }
+
+ #[test]
+ fn test_owned_pointer_default() {
+ let s: String = Default::default();
+ let foreign: OwnedPointer<String> = Default::default();
+ let native = foreign.into_native();
+ assert_eq!(s, native);
+ }
+
+ #[test]
+ fn test_owned_pointer_into() {
+ let s = "Hello, world!".to_string();
+ let cloned: OwnedPointer<String> = s.clone_to_foreign().into();
+ let copy = cloned.into_native();
+ assert_eq!(s, copy);
+ }
+
+ #[test]
+ fn test_owned_pointer_into_native() {
+ let s = "Hello, world!".to_string();
+ let cloned = s.clone_to_foreign();
+ let copy = cloned.into_native();
+ assert_eq!(s, copy);
+ }
+
+ #[test]
+ fn test_ptr_into_native() {
+ let s = "Hello, world!".to_string();
+ let cloned = s.clone_to_foreign_ptr();
+ let copy: String = unsafe { cloned.into_native() };
+ assert_eq!(s, copy);
+
+ // This is why type bounds are needed... they aren't for
+ // OwnedPointer::into_native
+ let cloned = s.clone_to_foreign_ptr();
+ let copy: c_char = unsafe { cloned.into_native() };
+ assert_eq!(s.as_bytes()[0], copy as u8);
+ }
+
+ #[test]
+ fn test_clone_to_foreign_str() {
+ let s = "Hello, world!";
+ let p = c"Hello, world!".as_ptr();
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ let len = libc::strlen(cloned.as_ptr());
+ assert_eq!(len, s.len());
+ assert_eq!(
+ libc::memcmp(
+ cloned.as_ptr() as *const c_void,
+ p as *const c_void,
+ len + 1
+ ),
+ 0
+ );
+ }
+ }
+
+ #[test]
+ fn test_clone_to_foreign_cstr() {
+ let s: &CStr = c"Hello, world!";
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ let len = libc::strlen(cloned.as_ptr());
+ assert_eq!(len, s.count_bytes());
+ assert_eq!(
+ libc::memcmp(
+ cloned.as_ptr() as *const c_void,
+ s.as_ptr() as *const c_void,
+ len + 1
+ ),
+ 0
+ );
+ }
+ }
+
+ #[test]
+ fn test_clone_to_foreign_string_cow() {
+ let p = c"Hello, world!".as_ptr();
+ for s in vec![
+ Into::<Cow<str>>::into("Hello, world!"),
+ Into::<Cow<str>>::into("Hello, world!".to_string())] {
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ let len = libc::strlen(cloned.as_ptr());
+ assert_eq!(len, s.len());
+ assert_eq!(
+ libc::memcmp(
+ cloned.as_ptr() as *const c_void,
+ p as *const c_void,
+ len + 1
+ ),
+ 0
+ );
+ }
+ }
+ }
+
+ #[test]
+ fn test_clone_to_foreign_bytes() {
+ let s = b"Hello, world!\0";
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ let len = libc::strlen(cloned.as_ptr() as *const c_char);
+ assert_eq!(len, s.len() - 1);
+ assert_eq!(
+ libc::memcmp(
+ cloned.as_ptr() as *const c_void,
+ s.as_ptr() as *const c_void,
+ len + 1
+ ),
+ 0
+ );
+ }
+ }
+
+ #[test]
+ fn test_clone_to_foreign_cstring() {
+ let s = CString::new("Hello, world!").unwrap();
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ assert_ne!(s.as_ptr(), cloned.as_ptr());
+ assert_eq!(
+ libc::strcmp(
+ cloned.as_ptr() as *const c_char,
+ s.as_ptr() as *const c_char,
+ ),
+ 0
+ );
+ }
+ }
+
+ #[test]
+ fn test_clone_to_foreign_string() {
+ let s = "Hello, world!".to_string();
+ let cstr = c"Hello, world!";
+ let cloned = s.clone_to_foreign();
+ unsafe {
+ let len = libc::strlen(cloned.as_ptr());
+ assert_eq!(len, s.len());
+ assert_eq!(
+ libc::memcmp(
+ cloned.as_ptr() as *const c_void,
+ cstr.as_ptr() as *const c_void,
+ len + 1
+ ),
+ 0
+ );
+ }
+ }
+
+ #[test]
+ fn test_option() {
+ // An Option can be used to produce or convert NULL pointers
+ let s = Some("Hello, world!".to_string());
+ let cstr = c"Hello, world!";
+ unsafe {
+ assert_eq!(Option::<String>::cloned_from_foreign(cstr.as_ptr()), s);
+ assert_matches!(Option::<String>::cloned_from_foreign(ptr::null()), None);
+ assert_matches!(Option::<String>::from_foreign(ptr::null_mut()), None);
+ }
+ }
+
+ #[test]
+ fn test_box() {
+ // A box can be produced if the inner type has the capability.
+ let s = Box::new("Hello, world!".to_string());
+ let cstr = c"Hello, world!";
+ let cloned = unsafe { Box::<String>::cloned_from_foreign(cstr.as_ptr()) };
+ assert_eq!(s, cloned);
+
+ let s = Some(Box::new("Hello, world!".to_string()));
+ let cloned = unsafe { Option::<Box<String>>::cloned_from_foreign(cstr.as_ptr()) };
+ assert_eq!(s, cloned);
+ }
+}
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 05/14] rust: define wrappers for Error
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (3 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 04/14] rust: add tests for util::foreign Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 06/14] rust: define wrappers for basic QOM concepts Paolo Bonzini
` (9 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
The wrappers for Error provide the following functionality:
- a struct that implements std::error::Error and can be filled with
information similar to what would go into a C Error* (location, message).
- functionality similar to error_prepend() via the "cause()"
member of std::error::Error.
- converting a Result<> into a value that can be returned to C,
filling in an Error** (like error_propagate() would do) if the
Result<> contains an error; useful for callbacks written in Rust
- converting a C Error* into a Result that can be returned to Rust,
useful for Rust wrappers of C functions
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/lib.rs | 2 +
qemu/src/util/error.rs | 241 +++++++++++++++++++++++++++++++++++++++++
qemu/src/util/mod.rs | 1 +
3 files changed, 244 insertions(+)
create mode 100644 qemu/src/util/error.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index c48edcf..5f926b8 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -4,7 +4,9 @@
pub mod bindings;
pub mod util;
+pub use util::error::Error;
pub use util::foreign::CloneToForeign;
pub use util::foreign::FromForeign;
pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
+pub type Result<T> = std::result::Result<T, Error>;
diff --git a/qemu/src/util/error.rs b/qemu/src/util/error.rs
new file mode 100644
index 0000000..e7e6f2e
--- /dev/null
+++ b/qemu/src/util/error.rs
@@ -0,0 +1,241 @@
+//! Error class for QEMU Rust code
+//!
+//! @author Paolo Bonzini
+
+use crate::bindings;
+use crate::bindings::error_free;
+use crate::bindings::error_get_pretty;
+use crate::bindings::error_setg_internal;
+
+use std::ffi::CStr;
+use std::fmt::{self, Display};
+use std::ptr;
+
+use crate::util::foreign::{CloneToForeign, FromForeign, OwnedPointer};
+
+#[derive(Debug, Default)]
+pub struct Error {
+ msg: Option<String>,
+ /// Appends the print string of the error to the msg if not None
+ cause: Option<Box<dyn std::error::Error>>,
+ location: Option<(String, u32)>,
+}
+
+impl std::error::Error for Error {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ self.cause.as_deref()
+ }
+
+ #[allow(deprecated)]
+ fn description(&self) -> &str {
+ self.msg
+ .as_deref()
+ .or_else(|| self.cause.as_deref().map(std::error::Error::description))
+ .unwrap_or("error")
+ }
+}
+
+impl Display for Error {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ let mut prefix = "";
+ if let Some((ref file, line)) = self.location {
+ write!(f, "{}:{}", file, line)?;
+ prefix = ": ";
+ }
+ if let Some(ref msg) = self.msg {
+ write!(f, "{}{}", prefix, msg)?;
+ prefix = ": ";
+ }
+ if let Some(ref cause) = self.cause {
+ write!(f, "{}{}", prefix, cause)?;
+ } else if prefix.is_empty() {
+ f.write_str("unknown error")?;
+ }
+ Ok(())
+ }
+}
+
+impl From<&str> for Error {
+ fn from(msg: &str) -> Self {
+ Error {
+ msg: Some(String::from(msg)),
+ cause: None,
+ location: None,
+ }
+ }
+}
+
+impl From<std::io::Error> for Error {
+ fn from(error: std::io::Error) -> Self {
+ Error {
+ msg: None,
+ cause: Some(Box::new(error)),
+ location: None,
+ }
+ }
+}
+
+impl Error {
+ /// Create a new error, prepending `msg` to the
+ /// description of `cause`
+ pub fn with_error<E: std::error::Error + 'static>(msg: &str, cause: E) -> Self {
+ Error {
+ msg: Some(String::from(msg)),
+ cause: Some(Box::new(cause)),
+ location: None,
+ }
+ }
+
+ /// Create a new error, prepending `file:line: msg` to the
+ /// description of `cause`
+ pub fn with_error_file_line<E: std::error::Error + 'static>(
+ msg: &str,
+ cause: E,
+ file: &str,
+ line: u32,
+ ) -> Self {
+ Error {
+ msg: Some(String::from(msg)),
+ cause: Some(Box::new(cause)),
+ location: Some((String::from(file), line)),
+ }
+ }
+
+ /// Create a new error with format `file:line: msg`
+ pub fn with_file_line(msg: &str, file: &str, line: u32) -> Self {
+ Error {
+ msg: Some(String::from(msg)),
+ cause: None,
+ location: Some((String::from(file), line)),
+ }
+ }
+
+ /// Consume a result, returning false if it is an error and
+ /// true if it is successful. The error is propagated into
+ /// `errp` like the C API `error_propagate` would do.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be valid; typically it is received from C code
+ pub unsafe fn bool_or_propagate<T>(
+ result: Result<(), Self>,
+ errp: *mut *mut bindings::Error,
+ ) -> bool {
+ Self::ok_or_propagate(result, errp).is_some()
+ }
+
+ /// Consume a result, returning a `NULL` pointer if it is an
+ /// error and a C representation of the contents if it is
+ /// successful. The error is propagated into `errp` like
+ /// the C API `error_propagate` would do.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be valid; typically it is received from C code
+ pub unsafe fn ptr_or_propagate<T: CloneToForeign>(
+ result: Result<T, Self>,
+ errp: *mut *mut bindings::Error,
+ ) -> *mut T::Foreign {
+ Self::ok_or_propagate(result, errp).map_or(ptr::null_mut(), |ref x| {
+ CloneToForeign::clone_to_foreign_ptr(x)
+ })
+ }
+
+ /// Consume a result and return `self.ok()`, but also propagate a
+ /// possible error into `errp`, like the C API `error_propagate`
+ /// would do.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be valid; typically it is received from C code
+ pub unsafe fn ok_or_propagate<T>(
+ result: Result<T, Self>,
+ errp: *mut *mut bindings::Error,
+ ) -> Option<T> {
+ match result {
+ Ok(ok) => Some(ok),
+ Err(err) => {
+ err.propagate(errp);
+ None
+ }
+ }
+ }
+
+ /// Equivalent of the C function `error_propagate`. Fill `*errp`
+ /// with the information container in `self` if `errp` is not NULL;
+ /// then consume it.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be valid; typically it is received from C code
+ pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
+ if errp.is_null() {
+ return;
+ }
+ errp.write(self.clone_to_foreign_ptr());
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, using `Ok(Default::default())`
+ /// if `c_error` is NULL.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be valid; typically it has been filled by a C
+ /// function.
+ pub unsafe fn err_or_default<T: Default>(c_error: *mut bindings::Error) -> Result<T, Self> {
+ Self::err_or_else(c_error, Default::default)
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, calling `f()` to
+ /// obtain an `Ok` value if `c_error` is NULL.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be valid; typically it has been filled by a C
+ /// function.
+ pub unsafe fn err_or_else<T, F: FnOnce() -> T>(
+ c_error: *mut bindings::Error,
+ f: F,
+ ) -> Result<T, Self> {
+ match Option::<Self>::from_foreign(c_error) {
+ None => Ok(f()),
+ Some(err) => Err(err),
+ }
+ }
+}
+
+impl CloneToForeign for Error {
+ type Foreign = bindings::Error;
+
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ let mut x: *mut bindings::Error = ptr::null_mut();
+ unsafe {
+ error_setg_internal(
+ &mut x,
+ ptr::null_mut(), // FIXME
+ 0,
+ ptr::null_mut(), // FIXME
+ c"%s".as_ptr(),
+ format!("{}", self),
+ );
+ OwnedPointer::new(x)
+ }
+ }
+
+ unsafe fn free_foreign(p: *mut bindings::Error) {
+ unsafe {
+ error_free(p);
+ }
+ }
+}
+
+impl FromForeign for Error {
+ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
+ let c_str = unsafe { CStr::from_ptr(error_get_pretty(c_error)) };
+ Error {
+ msg: Some(c_str.to_string_lossy().into_owned()),
+ cause: None,
+ location: None,
+ }
+ }
+}
diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
index be00d0c..e6078ac 100644
--- a/qemu/src/util/mod.rs
+++ b/qemu/src/util/mod.rs
@@ -1 +1,2 @@
+pub mod error;
pub mod foreign;
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 06/14] rust: define wrappers for basic QOM concepts
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (4 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 05/14] rust: define wrappers for Error Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 07/14] rust: define wrappers for methods of the QOM Object class Paolo Bonzini
` (8 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
This provides type-safe object casts, and automatic reference counting.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/qom-rust.txt | 82 ++++++++++++
qemu/src/lib.rs | 6 +
qemu/src/qom/mod.rs | 2 +
qemu/src/qom/object.rs | 34 +++++
qemu/src/qom/refs.rs | 274 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 398 insertions(+)
create mode 100644 qemu/qom-rust.txt
create mode 100644 qemu/src/qom/mod.rs
create mode 100644 qemu/src/qom/object.rs
create mode 100644 qemu/src/qom/refs.rs
diff --git a/qemu/qom-rust.txt b/qemu/qom-rust.txt
new file mode 100644
index 0000000..1588445
--- /dev/null
+++ b/qemu/qom-rust.txt
@@ -0,0 +1,82 @@
+Rust QOM interoperability design
+--------------------------------
+
+Passing objects around
+----------------------
+
+ObjectRef:
+-> trait for performing casts on objects
+-> upcasts safe at compile time, downcasts safe at runtime
+-> implemented by &T and qom::Owned<T>
+-> casting &T produces &U, casting qom::Owned<T> produces qom::Owned<U>
+
+qom::Owned<T>
+-> T is a struct for a QOM object
+-> cloning qom::Owned calls object_ref, dropping qom::Owned calls object_unref
+
+
+Calling methods
+---------------
+
+- all methods &self (interior mutability)
+ - Rust implementation needs to wrap state with Cell<>, RefCell<> or Mutex<>
+
+one struct per class; one trait per non-final class; one trait per interface
+struct: Object, Device, ...
+- defines constructors
+ example: PL011::new() (technically defined on ObjectType)
+
+- defines methods of final classes
+
+trait: ObjectMethods, DeviceMethods, UserCreatableMethods, ...
+- defines methods of non-final classes and interfaces
+ example: obj.typename()
+
+- automatically implemented by &T where T is a subclass
+
+
+all methods expect interior mutability
+- structs not Send/Sync by default since they contain C pointers
+ - hence &T and Owned<T> also not thread-safe
+- good: QOM tree (e.g. object_unparent) not thread-safe
+- what if objects _are_ thread-safe?
+ - possibly another trait ObjectSyncMethods?
+
+Bindings for C classes
+----------------------
+
+struct must implement ObjectType
+
+ unsafe impl ObjectType for Object {
+ const TYPE: &'static CStr = c"object";
+ }
+
+struct must implement IsA<T> for all superclasses T
+
+ unsafe impl IsA<Object> for Object {}
+
+
+Defining QOM classes in Rust
+----------------------------
+
+struct must be #[repr(C)]
+
+one traits per class + one more if it has virtual functions
+
+trait #1: ObjectTypeImpl, DeviceTypeImpl, ...
+- metadata
+ type Super: ObjectType;
+- functions:
+ unsafe fn instance_init(obj: *mut Self);
+ ...
+
+trait #2: ObjectImpl, DeviceImpl, ...
+- functions:
+ fn unrealize(&self)
+
+Rust implementation split in configuration (Default + ConstDefault) and
+state (Default)
+
+instance_init implemented automatically via Default/ConstDefault trait
+ maybe: pre_init hook that replaces memset(obj, 0, type->instance_size)?
+instance_finalize implemented automatically via Drop trait
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 5f926b8..0d91623 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,6 +2,12 @@
#![allow(dead_code)]
pub mod bindings;
+pub use bindings::Object;
+
+pub mod qom;
+pub use qom::object::ObjectType;
+pub use qom::refs::ObjectCast;
+pub use qom::refs::Owned;
pub mod util;
pub use util::error::Error;
diff --git a/qemu/src/qom/mod.rs b/qemu/src/qom/mod.rs
new file mode 100644
index 0000000..95489c5
--- /dev/null
+++ b/qemu/src/qom/mod.rs
@@ -0,0 +1,2 @@
+pub mod object;
+pub mod refs;
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
new file mode 100644
index 0000000..bd6b957
--- /dev/null
+++ b/qemu/src/qom/object.rs
@@ -0,0 +1,34 @@
+//! Bindings for the QOM Object class
+//!
+//! @author Paolo Bonzini
+
+use std::ffi::CStr;
+
+use crate::bindings::Object;
+
+use crate::qom_isa;
+
+/// Trait exposed by all structs corresponding to QOM objects.
+/// Defines "class methods" for the class. Usually these can be
+/// implemented on the class itself; here, using a trait allows
+/// each class to define `TYPE`, and it also lets `new()` return the
+/// right type.
+///
+/// # Safety
+///
+/// - the first field of the struct must be of `Object` type,
+/// or derived from it
+///
+/// - `TYPE` must match the type name used in the `TypeInfo` (no matter
+/// if it is defined in C or Rust).
+///
+/// - the struct must be `#[repr(C)]`
+pub unsafe trait ObjectType: Sized {
+ const TYPE: &'static CStr;
+}
+
+unsafe impl ObjectType for Object {
+ const TYPE: &'static CStr = c"object";
+}
+
+qom_isa!(Object);
diff --git a/qemu/src/qom/refs.rs b/qemu/src/qom/refs.rs
new file mode 100644
index 0000000..a319bde
--- /dev/null
+++ b/qemu/src/qom/refs.rs
@@ -0,0 +1,274 @@
+//! Casting and reference counting traits for QOM objects
+//!
+//! @author Paolo Bonzini
+
+use crate::bindings::object_dynamic_cast;
+use crate::bindings::Object;
+use crate::bindings::{object_ref, object_unref};
+
+use crate::qom::object::ObjectType;
+
+use std::borrow::Borrow;
+use std::mem::ManuallyDrop;
+use std::ops::Deref;
+use std::ptr::NonNull;
+
+/// Marker trait: `Self` can be statically upcasted to `P` (i.e. `P` is a direct
+/// or indirect parent of `Self`).
+///
+/// # Safety
+///
+/// The struct `Self` must begin, directly or indirectly, with a field of type
+/// `P`. This ensures that invalid casts, which rely on `IsA<>` for static
+/// checking, are rejected at compile time.
+pub unsafe trait IsA<P: ObjectType>: ObjectType {}
+
+// SAFETY: it is always safe to cast to your own type
+unsafe impl<T: ObjectType> IsA<T> for T {}
+
+#[macro_export]
+macro_rules! qom_isa {
+ ($struct:ty $(,$parent:ty)* ) => {
+ $(
+ impl AsRef<$parent> for $struct {
+ fn as_ref(&self) -> &$parent {
+ use $crate::ObjectCast;
+ self.upcast::<$parent>()
+ }
+ }
+
+ // SAFETY: it is the caller responsibility to have $parent as the
+ // first field
+ unsafe impl $crate::qom::refs::IsA<$parent> for $struct {}
+ )*
+ };
+}
+
+/// Trait for a reference to a QOM object. Allows conversion to/from
+/// C objects in generic code.
+pub trait ObjectCast: Copy + Deref
+where
+ Self::Target: ObjectType,
+{
+ /// Convert this (possibly smart) reference to a basic Rust reference.
+ fn as_ref(&self) -> &Self::Target {
+ self.deref()
+ }
+
+ /// Convert to a const Rust pointer, to be used for example for FFI.
+ fn as_ptr(&self) -> *const Self::Target {
+ self.as_ref()
+ }
+
+ /// Convert to a mutable Rust pointer, to be used for example for FFI.
+ /// Used to implement interior mutability for objects.
+ ///
+ /// # Safety
+ ///
+ /// This method is unsafe because it overrides const-ness of `&self`.
+ /// Bindings to C APIs will use it a lot, but otherwise it should not
+ /// be necessary.
+ unsafe fn as_mut_ptr(&self) -> *mut Self::Target {
+ #[allow(clippy::as_ptr_cast_mut)]
+ {
+ self.as_ptr().cast_mut()
+ }
+ }
+
+ /// Perform a cast to a superclass
+ fn upcast<'a, U: ObjectType>(self) -> &'a U
+ where
+ Self::Target: IsA<U>,
+ Self: 'a,
+ {
+ // SAFETY: soundness is declared via IsA<U>, which is an unsafe trait
+ unsafe { self.unsafe_cast::<U>() }
+ }
+
+ /// Perform a cast to a subclass. Checks at compile time that the
+ /// cast can succeed, but the final verification will happen at
+ /// runtime only.
+ fn downcast<'a, U: IsA<Self::Target>>(self) -> Option<&'a U>
+ where
+ Self: 'a,
+ {
+ self.dynamic_cast::<U>()
+ }
+
+ /// Perform a cast between QOM types. The check that U is indeed
+ /// the dynamic type of `self` happens at runtime.
+ fn dynamic_cast<'a, U: ObjectType>(self) -> Option<&'a U>
+ where
+ Self: 'a,
+ {
+ unsafe {
+ // SAFETY: upcasting to Object is always valid, and the
+ // return type is either NULL or the argument itself
+ let result: *const U =
+ object_dynamic_cast(self.unsafe_cast::<Object>().as_mut_ptr(), U::TYPE.as_ptr())
+ .cast();
+
+ result.as_ref()
+ }
+ }
+
+ /// Unconditional cast to an arbitrary QOM type.
+ ///
+ /// # Safety
+ ///
+ /// What safety? You need to know yourself that the cast is correct; only use
+ /// when performance is paramount. It is still better than a raw pointer
+ /// `cast()`, which does not even check that you remain in the realm of
+ /// QOM `ObjectType`s.
+ ///
+ /// `unsafe_cast::<Object>()` can also be used, and is always safe, if all
+ /// you have is an `ObjectType` (as opposed to an `IsA<Object>`).
+ unsafe fn unsafe_cast<'a, U: ObjectType>(self) -> &'a U
+ where
+ Self: 'a,
+ {
+ &*(self.as_ptr().cast::<U>())
+ }
+}
+
+impl<T: ObjectType> ObjectCast for &T {}
+
+/// An owned reference to a QOM object.
+///
+/// Like [`std::sync::Arc`], references are added with [`Clone::clone`] and removed
+/// by dropping the `Owned`.
+#[derive(PartialEq, Eq, Hash, PartialOrd, Ord)]
+pub struct Owned<T: ObjectType>(NonNull<T>);
+
+// QOM knows how to handle reference counting across threads, but sending
+// the Owned to another thread requires the implementation itself to be
+// thread-safe (aka Sync). But I'm not entirely sure that this is enough
+// (see for example ARef in rust/kernel/types.rs, which is very similar
+// to this type).
+//
+//unsafe impl<T: Sync + ObjectType> Send for Owned<T> {}
+//unsafe impl<T: ObjectType> Sync for Owned<T> {}
+
+impl<T: ObjectType> Owned<T> {
+ /// Obtain a reference from a raw C pointer
+ ///
+ /// # Safety
+ ///
+ /// Typically this function will only be used by low level bindings
+ /// to C APIs.
+ pub unsafe fn from_raw(ptr: *const T) -> Self {
+ // SAFETY NOTE: while NonNull requires a mutable pointer,
+ // only Deref is implemented so the pointer passed to from_raw
+ // remains const
+ Owned(NonNull::new_unchecked(ptr.cast_mut()))
+ }
+
+ /// Obtain a raw C pointer from a reference. `src` is consumed
+ /// and the reference is leaked.
+ pub fn into_raw(src: Owned<T>) -> *const T {
+ let src = ManuallyDrop::new(src);
+ src.0.as_ptr()
+ }
+
+
+ /// Increase the reference count of a QOM object and return
+ ///
+ /// # Safety
+ ///
+ /// Unsafe because the object could be embedded in another. To
+ /// obtain an `Owned` safely, use `ObjectType::new()`.
+ pub unsafe fn from(obj: &T) -> Self {
+ object_ref(obj.unsafe_cast::<Object>().as_mut_ptr());
+
+ // SAFETY NOTE: while NonNull requires a mutable pointer,
+ // only Deref is implemented so the pointer passed to from_raw
+ // remains const
+ Owned(NonNull::new_unchecked(obj.as_mut_ptr()))
+ }
+
+ /// Perform a cast to a superclass
+ pub fn upcast<U: ObjectType>(src: Owned<T>) -> Owned<U>
+ where
+ T: IsA<U>,
+ {
+ // SAFETY: soundness is declared via IsA<U>, which is an unsafe trait
+ unsafe { Owned::unsafe_cast::<U>(src) }
+ }
+
+ /// Perform a cast to a subclass. Checks at compile time that the
+ /// cast can succeed, but the final verification will happen at
+ /// runtime only.
+ pub fn downcast<U: IsA<T>>(src: Owned<T>) -> Result<Owned<U>, Owned<T>> {
+ Owned::dynamic_cast::<U>(src)
+ }
+
+ /// Perform a cast between QOM types. The check that U is indeed
+ /// the dynamic type of `self` happens at runtime.
+ pub fn dynamic_cast<U: ObjectType>(src: Owned<T>) -> Result<Owned<U>, Owned<T>> {
+ // override automatic drop to skip the unref/ref
+ let src = ManuallyDrop::new(src);
+ match src.dynamic_cast::<U>() {
+ // get the ownership back from the ManuallyDrop<>
+ None => Err(ManuallyDrop::into_inner(src)),
+
+ // SAFETY: the ref is moved (thanks to ManuallyDrop) from
+ // self to casted_ref
+ Some(casted_ref) => Ok(unsafe { Owned::<U>::from_raw(casted_ref) }),
+ }
+ }
+
+ /// Unconditional cast to an arbitrary QOM type.
+ ///
+ /// # Safety
+ ///
+ /// What safety? You need to know yourself that the cast is correct. Only use
+ /// when performance is paramount
+ pub unsafe fn unsafe_cast<U: ObjectType>(src: Owned<T>) -> Owned<U> {
+ // override automatic drop to skip the unref/ref
+ let src = ManuallyDrop::new(src);
+ let casted_ref = src.unsafe_cast::<U>();
+ Owned::<U>::from_raw(casted_ref)
+ }
+}
+
+impl<T: ObjectType> AsRef<T> for Owned<T> {
+ fn as_ref(&self) -> &T {
+ self.deref()
+ }
+}
+
+impl<T: ObjectType> Borrow<T> for Owned<T> {
+ fn borrow(&self) -> &T {
+ self.deref()
+ }
+}
+
+impl<T: ObjectType> Clone for Owned<T> {
+ fn clone(&self) -> Self {
+ // SAFETY: creation method is unsafe, and whoever calls it
+ // has responsibility that the pointer is valid
+ unsafe { Owned::from(self.deref()) }
+ }
+}
+
+impl<T: ObjectType> Deref for Owned<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: creation method is unsafe, and whoever calls it
+ // has responsibility that the pointer has a static lifetime.
+ // Once that is guaranteed, reference counting ensures that
+ // the object remains alive.
+ unsafe { &*self.0.as_ptr() }
+ }
+}
+
+impl<T: ObjectType> Drop for Owned<T> {
+ fn drop(&mut self) {
+ // SAFETY: creation method is unsafe, and whoever calls it
+ // has responsibility that the pointer is valid
+ unsafe {
+ object_unref(self.unsafe_cast::<Object>().as_mut_ptr());
+ }
+ }
+}
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 07/14] rust: define wrappers for methods of the QOM Object class
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (5 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 06/14] rust: define wrappers for basic QOM concepts Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 08/14] rust: define wrappers for methods of the QOM Device class Paolo Bonzini
` (7 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Provide a trait that can be used to invoke methods of the QOM object
class. The trait extends Deref and has a blanket implementation for any
type that dereferences to IsA<Object>. This way, it can be used on any
struct that dereferences to Object or a subclass.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/lib.rs | 2 +
qemu/src/qom/object.rs | 92 ++++++++++++++++++++++++++++++++++++++++++
qemu/src/qom/refs.rs | 8 ++++
3 files changed, 102 insertions(+)
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 0d91623..a6e7b17 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -5,6 +5,8 @@ pub mod bindings;
pub use bindings::Object;
pub mod qom;
+pub use qom::object::ObjectClassMethods;
+pub use qom::object::ObjectMethods;
pub use qom::object::ObjectType;
pub use qom::refs::ObjectCast;
pub use qom::refs::Owned;
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
index bd6b957..4e84e29 100644
--- a/qemu/src/qom/object.rs
+++ b/qemu/src/qom/object.rs
@@ -2,12 +2,26 @@
//!
//! @author Paolo Bonzini
+use std::borrow::Cow;
use std::ffi::CStr;
+use std::fmt;
+use std::ops::Deref;
+use crate::bindings::object_get_typename;
+use crate::bindings::object_property_add_child;
+use crate::bindings::object_new;
+use crate::bindings::object_unparent;
use crate::bindings::Object;
use crate::qom_isa;
+use crate::qom::refs::IsA;
+use crate::qom::refs::ObjectCast;
+use crate::qom::refs::Owned;
+
+use crate::util::foreign::CloneToForeign;
+use crate::util::foreign::FromForeign;
+
/// Trait exposed by all structs corresponding to QOM objects.
/// Defines "class methods" for the class. Usually these can be
/// implemented on the class itself; here, using a trait allows
@@ -31,4 +45,82 @@ unsafe impl ObjectType for Object {
const TYPE: &'static CStr = c"object";
}
+// ------------------------------
+// Object class
+
qom_isa!(Object);
+
+/// Trait for class methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA<Object>`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+
+pub trait ObjectClassMethods: IsA<Object> {
+ /// Return a new reference counted instance of this class
+ fn new() -> Owned<Self> {
+ // SAFETY: the object created by object_new is allocated on
+ // the heap and has a reference count of 1
+ unsafe {
+ let obj = &*object_new(Self::TYPE.as_ptr());
+ Owned::from_raw(obj.unsafe_cast::<Self>())
+ }
+ }
+}
+
+/// Trait for methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA<Object>`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+pub trait ObjectMethods: Deref
+where
+ Self::Target: IsA<Object>,
+{
+ /// Return the name of the type of `self`
+ fn typename(&self) -> Cow<'_, str> {
+ let obj = self.upcast::<Object>();
+ // SAFETY: safety of this is the requirement for implementing IsA
+ // The result of the C API has static lifetime
+ unsafe {
+ Cow::cloned_from_foreign(object_get_typename(obj.as_mut_ptr()))
+ }
+ }
+
+ /// Add an object as a child of the receiver.
+ fn property_add_child<T: ObjectType>(&self, name: &str, child: Owned<T>)
+ {
+ let obj = self.upcast::<Object>();
+ let name = name.clone_to_foreign();
+ unsafe {
+ // SAFETY: casting to object is always safe even if `child`'s
+ // target type is an interface type
+ let child = child.unsafe_cast::<Object>();
+ object_property_add_child(obj.as_mut_ptr(),
+ name.as_ptr(),
+ child.as_mut_ptr());
+
+ // object_property_add_child() added a reference of its own;
+ // dropping the one in `child` is the common case.
+ }
+ }
+
+ /// Remove the object from the QOM tree
+ fn unparent(&self) {
+ let obj = self.upcast::<Object>();
+ // SAFETY: safety of this is the requirement for implementing IsA
+ unsafe {
+ object_unparent(obj.as_mut_ptr());
+ }
+ }
+
+ /// Convenience function for implementing the Debug trait
+ fn debug_fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ f.debug_tuple(&self.typename())
+ .field(&(self as *const Self))
+ .finish()
+ }
+}
+
+impl<R> ObjectClassMethods for R where R: IsA<Object> {}
+impl<R: Deref> ObjectMethods for R where R::Target: IsA<Object> {}
diff --git a/qemu/src/qom/refs.rs b/qemu/src/qom/refs.rs
index a319bde..431ef0a 100644
--- a/qemu/src/qom/refs.rs
+++ b/qemu/src/qom/refs.rs
@@ -6,9 +6,11 @@ use crate::bindings::object_dynamic_cast;
use crate::bindings::Object;
use crate::bindings::{object_ref, object_unref};
+use crate::qom::object::ObjectMethods;
use crate::qom::object::ObjectType;
use std::borrow::Borrow;
+use std::fmt::{self, Debug};
use std::mem::ManuallyDrop;
use std::ops::Deref;
use std::ptr::NonNull;
@@ -272,3 +274,9 @@ impl<T: ObjectType> Drop for Owned<T> {
}
}
}
+
+impl<T: IsA<Object>> Debug for Owned<T> {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ self.deref().debug_fmt(f)
+ }
+}
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 08/14] rust: define wrappers for methods of the QOM Device class
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (6 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 07/14] rust: define wrappers for methods of the QOM Object class Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 09/14] rust: add idiomatic bindings to define Object subclasses Paolo Bonzini
` (6 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Provide a trait that can be used to invoke methods of the QOM Device
class. The trait extends Deref and has a blanket implementation for any
type that dereferences to IsA<DeviceState>. This way, it can be used on any
struct that dereferences to Object or a subclass.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/hw/core/device.rs | 56 ++++++++++++++++++++++++++++++++++++++
qemu/src/hw/core/mod.rs | 1 +
qemu/src/hw/mod.rs | 1 +
qemu/src/lib.rs | 4 +++
4 files changed, 62 insertions(+)
create mode 100644 qemu/src/hw/core/device.rs
create mode 100644 qemu/src/hw/core/mod.rs
create mode 100644 qemu/src/hw/mod.rs
diff --git a/qemu/src/hw/core/device.rs b/qemu/src/hw/core/device.rs
new file mode 100644
index 0000000..294251e
--- /dev/null
+++ b/qemu/src/hw/core/device.rs
@@ -0,0 +1,56 @@
+//! Bindings for the QOM Device class
+//!
+//! @author Paolo Bonzini
+
+use crate::qom::object::ObjectType;
+
+use crate::qom::refs::IsA;
+use crate::qom::refs::ObjectCast;
+
+use crate::bindings;
+use crate::bindings::device_cold_reset;
+use crate::bindings::device_realize;
+use crate::bindings::DeviceState;
+use crate::bindings::Object;
+
+use crate::qom_isa;
+
+use crate::Result;
+
+use std::ffi::CStr;
+use std::ops::Deref;
+use std::ptr::null_mut;
+
+unsafe impl ObjectType for DeviceState {
+ const TYPE: &'static CStr = c"device";
+}
+
+qom_isa!(DeviceState, Object);
+
+/// Trait for methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA<Object>`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+pub trait DeviceMethods: Deref
+where
+ Self::Target: IsA<DeviceState>,
+{
+ fn realize(&self) -> Result<()> {
+ let device = self.upcast::<DeviceState>();
+ let mut err: *mut bindings::Error = null_mut();
+ // SAFETY: safety of this is the requirement for implementing IsA
+ unsafe {
+ device_realize(device.as_mut_ptr(), &mut err);
+ crate::Error::err_or_default(err)
+ }
+ }
+
+ fn cold_reset(&self) {
+ let device = self.upcast::<DeviceState>();
+ // SAFETY: safety of this is the requirement for implementing IsA
+ unsafe { device_cold_reset(device.as_mut_ptr()) }
+ }
+}
+
+impl<R: Deref> DeviceMethods for R where R::Target: IsA<DeviceState> {}
diff --git a/qemu/src/hw/core/mod.rs b/qemu/src/hw/core/mod.rs
new file mode 100644
index 0000000..5458924
--- /dev/null
+++ b/qemu/src/hw/core/mod.rs
@@ -0,0 +1 @@
+pub mod device;
diff --git a/qemu/src/hw/mod.rs b/qemu/src/hw/mod.rs
new file mode 100644
index 0000000..5a7ca06
--- /dev/null
+++ b/qemu/src/hw/mod.rs
@@ -0,0 +1 @@
+pub mod core;
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index a6e7b17..b0dcce1 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,8 +2,12 @@
#![allow(dead_code)]
pub mod bindings;
+pub use bindings::DeviceState;
pub use bindings::Object;
+pub mod hw;
+pub use hw::core::device::DeviceMethods;
+
pub mod qom;
pub use qom::object::ObjectClassMethods;
pub use qom::object::ObjectMethods;
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 09/14] rust: add idiomatic bindings to define Object subclasses
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (7 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 08/14] rust: define wrappers for methods of the QOM Device class Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 10/14] rust: add idiomatic bindings to define Device subclasses Paolo Bonzini
` (5 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Provide a macro to register a type and automatically define instance_init
(actually instance_mem_init) and instance_finalize functions. Subclasses
of Object must define a trait ObjectImpl, to point the type definition
machinery to the implementation of virtual functions in Object.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/lib.rs | 4 +
qemu/src/qom/mod.rs | 1 +
qemu/src/qom/object_impl.rs | 146 ++++++++++++++++++++++++++++++++++++
qemu/src/util/mod.rs | 1 +
qemu/src/util/zeroed.rs | 21 ++++++
qemu/tests/main.rs | 32 ++++++++
6 files changed, 205 insertions(+)
create mode 100644 qemu/src/qom/object_impl.rs
create mode 100644 qemu/src/util/zeroed.rs
create mode 100644 qemu/tests/main.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index b0dcce1..81abf9c 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -4,6 +4,7 @@
pub mod bindings;
pub use bindings::DeviceState;
pub use bindings::Object;
+pub use bindings::TypeInfo;
pub mod hw;
pub use hw::core::device::DeviceMethods;
@@ -12,6 +13,8 @@ pub mod qom;
pub use qom::object::ObjectClassMethods;
pub use qom::object::ObjectMethods;
pub use qom::object::ObjectType;
+pub use qom::object_impl::ObjectImpl;
+pub use qom::object_impl::TypeImpl;
pub use qom::refs::ObjectCast;
pub use qom::refs::Owned;
@@ -21,4 +24,5 @@ pub use util::foreign::CloneToForeign;
pub use util::foreign::FromForeign;
pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
+pub use util::zeroed::Zeroed;
pub type Result<T> = std::result::Result<T, Error>;
diff --git a/qemu/src/qom/mod.rs b/qemu/src/qom/mod.rs
index 95489c5..3f8ee6e 100644
--- a/qemu/src/qom/mod.rs
+++ b/qemu/src/qom/mod.rs
@@ -1,2 +1,3 @@
pub mod object;
+pub mod object_impl;
pub mod refs;
diff --git a/qemu/src/qom/object_impl.rs b/qemu/src/qom/object_impl.rs
new file mode 100644
index 0000000..61546b6
--- /dev/null
+++ b/qemu/src/qom/object_impl.rs
@@ -0,0 +1,146 @@
+//! Macros and traits to implement subclasses of Object in Rust
+//!
+//! @author Paolo Bonzini
+
+#![allow(clippy::missing_safety_doc)]
+
+use const_default::ConstDefault;
+
+use std::ffi::c_void;
+use std::mem;
+use std::mem::MaybeUninit;
+use std::ptr::drop_in_place;
+
+use crate::qom::object::ObjectType;
+
+use crate::qom::refs::ObjectCast;
+
+use crate::bindings::type_register;
+use crate::bindings::Object;
+use crate::bindings::ObjectClass;
+use crate::bindings::TypeInfo;
+
+use crate::util::zeroed::Zeroed;
+
+/// Information on which superclass methods are overridden
+/// by a Rust-implemented subclass of Object.
+pub trait ObjectImpl: ObjectType {
+ /// If not `None`, a function that implements the `unparent` member
+ /// of the QOM `ObjectClass`.
+ const UNPARENT: Option<fn(obj: &Self)> = None;
+}
+
+impl ObjectClass {
+ /// Initialize an `ObjectClass` from an `ObjectImpl`.
+ pub fn class_init<T: ObjectImpl>(&mut self) {
+ unsafe extern "C" fn rust_unparent<T: ObjectImpl>(obj: *mut Object) {
+ let f = T::UNPARENT.unwrap();
+ f((&*obj).unsafe_cast::<T>())
+ }
+ self.unparent = T::UNPARENT.map(|_| rust_unparent::<T> as _);
+ }
+}
+
+impl Object {
+ pub unsafe extern "C" fn rust_class_init<T: ObjectImpl>(
+ klass: *mut c_void,
+ _data: *mut c_void,
+ ) {
+ let oc: &mut ObjectClass = &mut *(klass.cast());
+ oc.class_init::<T>();
+ }
+}
+
+/// Internal information on a Rust-implemented subclass of Object.
+/// Only public because it is used by macros.
+pub unsafe trait TypeImpl: ObjectType + ObjectImpl {
+ type Super: ObjectType;
+ type Conf: ConstDefault;
+ type State: Default;
+
+ const CLASS_INIT: unsafe extern "C" fn(klass: *mut c_void, data: *mut c_void);
+
+ fn uninit_conf(obj: &mut MaybeUninit<Self>) -> &mut MaybeUninit<Self::Conf>;
+ fn uninit_state(obj: &mut MaybeUninit<Self>) -> &mut MaybeUninit<Self::State>;
+}
+
+unsafe fn rust_type_register<T: TypeImpl + ObjectImpl>() {
+ unsafe extern "C" fn rust_instance_mem_init<T: TypeImpl>(obj: *mut c_void) {
+ let obj: &mut std::mem::MaybeUninit<T> = &mut *(obj.cast());
+
+ T::uninit_conf(obj).write(ConstDefault::DEFAULT);
+ T::uninit_state(obj).write(Default::default());
+ }
+
+ unsafe extern "C" fn rust_instance_finalize<T: TypeImpl>(obj: *mut c_void) {
+ let obj: *mut T = obj.cast();
+ drop_in_place(obj);
+ }
+
+ let ti = TypeInfo {
+ name: T::TYPE.as_ptr(),
+ parent: T::Super::TYPE.as_ptr(),
+ instance_size: mem::size_of::<T>(),
+ instance_mem_init: Some(rust_instance_mem_init::<T>),
+ instance_finalize: Some(rust_instance_finalize::<T>),
+ class_init: Some(T::CLASS_INIT),
+
+ // SAFETY: TypeInfo is defined in C and all fields are okay to be zeroed
+ ..Zeroed::zeroed()
+ };
+
+ type_register(&ti)
+}
+
+#[macro_export]
+macro_rules! qom_define_type {
+ ($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty; @extends $super:ty $(,$supers:ty)*) => {
+ struct $struct {
+ // self.base dropped by call to superclass instance_finalize
+ base: std::mem::ManuallyDrop<$super>,
+ conf: $conf_ty,
+ state: $state_ty,
+ }
+
+ // Define IsA markers for the struct itself and all the superclasses
+ $crate::qom_isa!($struct, $super $(,$supers)*);
+
+ unsafe impl $crate::qom::object::ObjectType for $struct {
+ const TYPE: &'static std::ffi::CStr = $name;
+ }
+
+ unsafe impl $crate::qom::object_impl::TypeImpl for $struct {
+ type Super = $super;
+ type Conf = $conf_ty;
+ type State = $state_ty;
+
+ const CLASS_INIT: unsafe extern "C" fn(klass: *mut std::ffi::c_void, data: *mut std::ffi::c_void)
+ = <$super>::rust_class_init::<Self>;
+
+ fn uninit_conf(obj: &mut std::mem::MaybeUninit::<Self>) -> &mut std::mem::MaybeUninit<$conf_ty> {
+ use std::ptr::addr_of_mut;
+
+ // Projecting the incoming reference to a single field is safe,
+ // because the return value is also MaybeUnit
+ unsafe { &mut *(addr_of_mut!((*obj.as_mut_ptr()).conf).cast()) }
+ }
+
+ fn uninit_state(obj: &mut std::mem::MaybeUninit::<Self>) -> &mut std::mem::MaybeUninit<$state_ty> {
+ use std::ptr::addr_of_mut;
+
+ // Projecting the incoming reference to a single field is safe,
+ // because the return value is also MaybeUnit
+ unsafe { &mut *(addr_of_mut!((*obj.as_mut_ptr()).state).cast()) }
+ }
+ }
+
+ // TODO: call rust_type_register
+ };
+}
+
+#[macro_export]
+macro_rules! conf_type {
+ ($type:ty) => {
+ <$type as $crate::qom::object_impl::TypeImpl>::Conf
+ };
+}
diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
index e6078ac..9c081b6 100644
--- a/qemu/src/util/mod.rs
+++ b/qemu/src/util/mod.rs
@@ -1,2 +1,3 @@
pub mod error;
pub mod foreign;
+pub mod zeroed;
diff --git a/qemu/src/util/zeroed.rs b/qemu/src/util/zeroed.rs
new file mode 100644
index 0000000..e656834
--- /dev/null
+++ b/qemu/src/util/zeroed.rs
@@ -0,0 +1,21 @@
+#![allow(clippy::undocumented_unsafe_blocks)]
+
+use std::mem::MaybeUninit;
+
+/// Trait providing an easy way to obtain an all-zero
+/// value for a struct
+///
+/// # Safety
+///
+/// Only add this to a type if `MaybeUninit::zeroed().assume_init()`
+/// is valid for that type.
+pub unsafe trait Zeroed: Sized {
+ fn zeroed() -> Self {
+ // SAFETY: If this weren't safe, just do not add the
+ // trait to a type.
+ unsafe { MaybeUninit::zeroed().assume_init() }
+ }
+}
+
+// Put here all the impls that you need for the bindgen-provided types.
+unsafe impl Zeroed for crate::bindings::TypeInfo {}
diff --git a/qemu/tests/main.rs b/qemu/tests/main.rs
new file mode 100644
index 0000000..a7cbeed
--- /dev/null
+++ b/qemu/tests/main.rs
@@ -0,0 +1,32 @@
+use const_default::ConstDefault;
+
+use qemu::qom_define_type;
+use qemu::Object;
+use qemu::ObjectClassMethods;
+use qemu::ObjectImpl;
+
+#[derive(Default, ConstDefault)]
+struct TestConf {
+ #[allow(dead_code)]
+ foo: bool,
+}
+
+#[derive(Default)]
+struct TestState {
+ #[allow(dead_code)]
+ bar: i32,
+}
+
+qom_define_type!(
+ c"test-object",
+ TestObject,
+ TestConf,
+ ();
+ @extends Object
+);
+
+impl ObjectImpl for TestObject {}
+
+fn main() {
+ drop(TestObject::new());
+}
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 10/14] rust: add idiomatic bindings to define Device subclasses
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (8 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 09/14] rust: add idiomatic bindings to define Object subclasses Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 11/14] rust: replace std::ffi::c_char with libc::c_char Paolo Bonzini
` (4 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Provide a macro to register a type and automatically define qdev
properties. Subclasses of DeviceState must define a trait DeviceImpl, to
point the type definition machinery to the implementation of virtual
functions in DeviceState.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/hw/core/device_impl.rs | 140 ++++++++++++++++++++++++++++++++
qemu/src/hw/core/mod.rs | 1 +
qemu/src/lib.rs | 5 ++
qemu/tests/main.rs | 52 +++++++++++-
4 files changed, 197 insertions(+), 1 deletion(-)
create mode 100644 qemu/src/hw/core/device_impl.rs
diff --git a/qemu/src/hw/core/device_impl.rs b/qemu/src/hw/core/device_impl.rs
new file mode 100644
index 0000000..80b0e5e
--- /dev/null
+++ b/qemu/src/hw/core/device_impl.rs
@@ -0,0 +1,140 @@
+//! Macros and traits to implement subclasses of Device in Rust
+//!
+//! @author Paolo Bonzini
+
+#![allow(clippy::missing_safety_doc)]
+
+use std::ffi::c_void;
+
+use crate::bindings;
+use crate::bindings::DeviceClass;
+use crate::bindings::DeviceState;
+use crate::bindings::Property;
+
+use crate::qom::object_impl::ObjectImpl;
+use crate::qom::object_impl::TypeImpl;
+
+use crate::qom::refs::ObjectCast;
+
+use crate::util::error::Error;
+
+/// Information on which superclass methods are overridden
+/// by a Rust-implemented subclass of Device.
+pub trait DeviceImpl: ObjectImpl + DeviceTypeImpl {
+ /// If not `None`, a function that implements the `realize` member
+ /// of the QOM `DeviceClass`.
+ const REALIZE: Option<fn(obj: &Self) -> crate::Result<()>> = None;
+
+ /// If not `None`, a function that implements the `unrealize` member
+ /// of the QOM `DeviceClass`.
+ const UNREALIZE: Option<fn(obj: &Self)> = None;
+
+ /// If not `None`, a function that implements the `cold_reset` member
+ /// of the QOM `DeviceClass`.
+ const COLD_RESET: Option<fn(obj: &Self)> = None;
+}
+
+impl DeviceClass {
+ pub fn class_init<T: DeviceImpl>(&mut self) {
+ unsafe extern "C" fn rust_cold_reset<T: DeviceImpl>(obj: *mut DeviceState) {
+ let f = T::COLD_RESET.unwrap();
+ f((&*obj).unsafe_cast::<T>())
+ }
+ self.cold_reset = T::COLD_RESET.map(|_| rust_cold_reset::<T> as _);
+
+ unsafe extern "C" fn rust_realize<T: DeviceImpl>(
+ obj: *mut DeviceState,
+ errp: *mut *mut bindings::Error,
+ ) {
+ let f = T::REALIZE.unwrap();
+ let result = f((&*obj).unsafe_cast::<T>());
+ Error::ok_or_propagate(result, errp);
+ }
+ self.realize = T::REALIZE.map(|_| rust_realize::<T> as _);
+
+ unsafe extern "C" fn rust_unrealize<T: DeviceImpl>(obj: *mut DeviceState) {
+ let f = T::UNREALIZE.unwrap();
+ f((&*obj).unsafe_cast::<T>())
+ }
+ self.unrealize = T::UNREALIZE.map(|_| rust_unrealize::<T> as _);
+
+ self.properties = <T as DeviceTypeImpl>::properties();
+
+ // Now initialize the ObjectClass from the ObjectImpl.
+ self.oc.class_init::<T>();
+ }
+}
+
+impl DeviceState {
+ pub unsafe extern "C" fn rust_class_init<T: DeviceImpl>(
+ klass: *mut c_void,
+ _data: *mut c_void,
+ ) {
+ let dc: &mut DeviceClass = &mut *(klass.cast());
+ dc.class_init::<T>();
+ }
+}
+
+/// Internal information on a Rust-implemented subclass of Device.
+/// Only public because it is used by macros.
+pub unsafe trait DeviceTypeImpl: TypeImpl {
+ const CONF_OFFSET: usize;
+
+ // This needs to be here, and not in DeviceImpl, because properties
+ // reference statics (for globals defined in C, e.g. qdev_prop_bool)
+ // which is unstable (see https://github.com/rust-lang/rust/issues/119618,
+ // feature const_refs_to_static)
+ fn properties() -> *const Property;
+}
+
+pub struct QdevPropBool;
+impl QdevPropBool {
+ pub const fn convert(value: &bool) -> u64 {
+ *value as u64
+ }
+}
+
+#[macro_export]
+macro_rules! qdev_prop {
+ (@internal bool, $name:expr, $default:expr, $offset:expr) => {
+ $crate::Property {
+ name: $name.as_ptr(),
+ offset: $offset,
+ default: $crate::hw::core::device_impl::QdevPropBool::convert(&($default)),
+ info: unsafe { &$crate::bindings::qdev_prop_bool },
+ }
+ };
+
+ // Replace field with typechecking expression and offset
+ ($kind:tt, $name:expr, $type:ty, $default:expr, $field:ident) => {
+ qdev_prop!(@internal
+ $kind,
+ $name,
+ (<$crate::conf_type!($type) as ConstDefault>::DEFAULT).$field,
+ <$type as $crate::DeviceTypeImpl>::CONF_OFFSET + std::mem::offset_of!($crate::conf_type!($type), $field)
+ )
+ };
+}
+
+#[macro_export]
+macro_rules! qdev_define_type {
+ ($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty;
+ @extends $super:ty $(,$supers:ty)*;
+ @properties [$($props: expr),+]) => {
+ $crate::qom_define_type!(
+ $name, $struct, $conf_ty, $state_ty;
+ @extends $super $(,$supers)*, $crate::Object);
+
+ unsafe impl $crate::DeviceTypeImpl for $struct {
+ const CONF_OFFSET: usize = std::mem::offset_of!($struct, conf);
+
+ fn properties() -> *const $crate::Property {
+ static mut PROPERTIES: &'static [$crate::Property] = &[$($props),+];
+
+ // SAFETY: The only reference is created here; mut is needed to refer to
+ // &qdev_prop_xxx.
+ unsafe { PROPERTIES.as_ptr() }
+ }
+ }
+ }
+}
diff --git a/qemu/src/hw/core/mod.rs b/qemu/src/hw/core/mod.rs
index 5458924..6cd9197 100644
--- a/qemu/src/hw/core/mod.rs
+++ b/qemu/src/hw/core/mod.rs
@@ -1 +1,2 @@
pub mod device;
+pub mod device_impl;
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 81abf9c..3f0491c 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,12 +2,17 @@
#![allow(dead_code)]
pub mod bindings;
+pub use bindings::DeviceClass;
pub use bindings::DeviceState;
pub use bindings::Object;
+pub use bindings::Property;
+pub use bindings::PropertyInfo;
pub use bindings::TypeInfo;
pub mod hw;
pub use hw::core::device::DeviceMethods;
+pub use hw::core::device_impl::DeviceImpl;
+pub use hw::core::device_impl::DeviceTypeImpl;
pub mod qom;
pub use qom::object::ObjectClassMethods;
diff --git a/qemu/tests/main.rs b/qemu/tests/main.rs
index a7cbeed..e499c14 100644
--- a/qemu/tests/main.rs
+++ b/qemu/tests/main.rs
@@ -5,9 +5,18 @@ use qemu::Object;
use qemu::ObjectClassMethods;
use qemu::ObjectImpl;
+use qemu::qdev_define_type;
+use qemu::qdev_prop;
+use qemu::DeviceImpl;
+use qemu::DeviceMethods;
+use qemu::DeviceState;
+
+use qemu::Result;
+
+use std::cell::RefCell;
+
#[derive(Default, ConstDefault)]
struct TestConf {
- #[allow(dead_code)]
foo: bool,
}
@@ -27,6 +36,47 @@ qom_define_type!(
impl ObjectImpl for TestObject {}
+qdev_define_type!(
+ c"test-device",
+ TestDevice,
+ TestConf,
+ RefCell<TestState>;
+ @extends DeviceState;
+ @properties [qdev_prop!(bool, c"foo", TestDevice, true, foo)]
+);
+
+impl TestDevice {
+ #[allow(clippy::unused_self)]
+ fn unparent(&self) {
+ println!("unparent");
+ }
+
+ #[allow(clippy::unused_self)]
+ fn realize(&self) -> Result<()> {
+ println!("realize");
+ Ok(())
+ }
+
+ #[allow(clippy::unused_self)]
+ fn unrealize(&self) {
+ println!("unrealize");
+ }
+}
+
+impl ObjectImpl for TestDevice {
+ const UNPARENT: Option<fn(&TestDevice)> = Some(TestDevice::unparent);
+}
+
+impl DeviceImpl for TestDevice {
+ const REALIZE: Option<fn(&TestDevice) -> Result<()>> = Some(TestDevice::realize);
+ const UNREALIZE: Option<fn(&TestDevice)> = Some(TestDevice::unrealize);
+}
+
fn main() {
drop(TestObject::new());
+
+ let d = TestDevice::new();
+ d.realize().unwrap();
+ d.cold_reset();
+ d.unparent();
}
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 11/14] rust: replace std::ffi::c_char with libc::c_char
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (9 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 10/14] rust: add idiomatic bindings to define Device subclasses Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 12/14] rust: replace c"" literals with cstr crate Paolo Bonzini
` (3 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Allow working with Rust versions prior to 1.64.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/src/bindings/mod.rs | 3 ++-
qemu/src/util/foreign.rs | 7 +++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/qemu/src/bindings/mod.rs b/qemu/src/bindings/mod.rs
index a49447b..0ad3828 100644
--- a/qemu/src/bindings/mod.rs
+++ b/qemu/src/bindings/mod.rs
@@ -1,4 +1,5 @@
-use std::ffi::{c_char, c_void};
+use libc::c_char;
+use std::ffi::c_void;
#[repr(C)]
pub struct Object {
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index 0b8b708..464400a 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -4,8 +4,11 @@
/// Traits to map between C structs and native Rust types.
/// Similar to glib-rs but a bit simpler and possibly more
/// idiomatic.
+
+use libc::c_char;
+
use std::borrow::Cow;
-use std::ffi::{c_char, c_void, CStr, CString};
+use std::ffi::{c_void, CStr, CString};
use std::fmt;
use std::fmt::Debug;
use std::mem;
@@ -166,7 +169,7 @@ pub trait FromForeign: CloneToForeign + Sized {
/// # use qemu::FromForeign;
/// let p = c"Hello, world!".as_ptr();
/// let s = unsafe {
- /// String::cloned_from_foreign(p as *const std::ffi::c_char)
+ /// String::cloned_from_foreign(p as *const libc::c_char)
/// };
/// assert_eq!(s, "Hello, world!");
/// ```
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 12/14] rust: replace c"" literals with cstr crate
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (10 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 11/14] rust: replace std::ffi::c_char with libc::c_char Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 13/14] rust: introduce alternative to offset_of! Paolo Bonzini
` (2 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Part of what's needed to work with Rust versions prior to 1.77.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/Cargo.toml | 3 +++
qemu/qom-rust.txt | 2 +-
qemu/src/hw/core/device.rs | 4 +++-
qemu/src/qom/object.rs | 4 +++-
qemu/src/util/error.rs | 4 +++-
qemu/src/util/foreign.rs | 20 +++++++++++---------
qemu/tests/main.rs | 7 ++++---
8 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 1100725..a07a449 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -7,5 +7,8 @@ edition = "2021"
const-default = { version = "~1", features = ["derive"] }
libc = "^0"
+# pick older version in order to support Rust 1.63
+cstr = { version = "=0.2.10" }
+
[dev-dependencies]
matches = ">=0"
diff --git a/qemu/qom-rust.txt b/qemu/qom-rust.txt
index 1588445..ef4bd06 100644
--- a/qemu/qom-rust.txt
+++ b/qemu/qom-rust.txt
@@ -48,7 +48,7 @@ Bindings for C classes
struct must implement ObjectType
unsafe impl ObjectType for Object {
- const TYPE: &'static CStr = c"object";
+ const TYPE: &'static CStr = cstr!("object");
}
struct must implement IsA<T> for all superclasses T
diff --git a/qemu/src/hw/core/device.rs b/qemu/src/hw/core/device.rs
index 294251e..4edf61d 100644
--- a/qemu/src/hw/core/device.rs
+++ b/qemu/src/hw/core/device.rs
@@ -17,12 +17,14 @@ use crate::qom_isa;
use crate::Result;
+use cstr::cstr;
+
use std::ffi::CStr;
use std::ops::Deref;
use std::ptr::null_mut;
unsafe impl ObjectType for DeviceState {
- const TYPE: &'static CStr = c"device";
+ const TYPE: &'static CStr = cstr!("device");
}
qom_isa!(DeviceState, Object);
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
index 4e84e29..9f6c078 100644
--- a/qemu/src/qom/object.rs
+++ b/qemu/src/qom/object.rs
@@ -7,6 +7,8 @@ use std::ffi::CStr;
use std::fmt;
use std::ops::Deref;
+use cstr::cstr;
+
use crate::bindings::object_get_typename;
use crate::bindings::object_property_add_child;
use crate::bindings::object_new;
@@ -42,7 +44,7 @@ pub unsafe trait ObjectType: Sized {
}
unsafe impl ObjectType for Object {
- const TYPE: &'static CStr = c"object";
+ const TYPE: &'static CStr = cstr!("object");
}
// ------------------------------
diff --git a/qemu/src/util/error.rs b/qemu/src/util/error.rs
index e7e6f2e..79c3c81 100644
--- a/qemu/src/util/error.rs
+++ b/qemu/src/util/error.rs
@@ -7,6 +7,8 @@ use crate::bindings::error_free;
use crate::bindings::error_get_pretty;
use crate::bindings::error_setg_internal;
+use cstr::cstr;
+
use std::ffi::CStr;
use std::fmt::{self, Display};
use std::ptr;
@@ -215,7 +217,7 @@ impl CloneToForeign for Error {
ptr::null_mut(), // FIXME
0,
ptr::null_mut(), // FIXME
- c"%s".as_ptr(),
+ cstr!("%s").as_ptr(),
format!("{}", self),
);
OwnedPointer::new(x)
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index 464400a..7a663cc 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -167,7 +167,8 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// ```
/// # use qemu::FromForeign;
- /// let p = c"Hello, world!".as_ptr();
+ /// # use cstr::cstr;
+ /// let p = cstr!("Hello, world!").as_ptr();
/// let s = unsafe {
/// String::cloned_from_foreign(p as *const libc::c_char)
/// };
@@ -476,6 +477,7 @@ mod tests {
#![allow(clippy::shadow_unrelated)]
use super::*;
+ use cstr::cstr;
use matches::assert_matches;
use std::ffi::c_void;
@@ -498,7 +500,7 @@ mod tests {
#[test]
fn test_cloned_from_foreign_string_cow() {
let s = "Hello, world!".to_string();
- let cstr = c"Hello, world!";
+ let cstr = cstr!("Hello, world!");
let cloned = unsafe { Cow::cloned_from_foreign(cstr.as_ptr()) };
assert_eq!(s, cloned);
}
@@ -506,7 +508,7 @@ mod tests {
#[test]
fn test_cloned_from_foreign_string() {
let s = "Hello, world!".to_string();
- let cstr = c"Hello, world!";
+ let cstr = cstr!("Hello, world!");
let cloned = unsafe { String::cloned_from_foreign(cstr.as_ptr()) };
assert_eq!(s, cloned);
}
@@ -570,7 +572,7 @@ mod tests {
#[test]
fn test_clone_to_foreign_str() {
let s = "Hello, world!";
- let p = c"Hello, world!".as_ptr();
+ let p = cstr!("Hello, world!").as_ptr();
let cloned = s.clone_to_foreign();
unsafe {
let len = libc::strlen(cloned.as_ptr());
@@ -588,7 +590,7 @@ mod tests {
#[test]
fn test_clone_to_foreign_cstr() {
- let s: &CStr = c"Hello, world!";
+ let s: &CStr = cstr!("Hello, world!");
let cloned = s.clone_to_foreign();
unsafe {
let len = libc::strlen(cloned.as_ptr());
@@ -606,7 +608,7 @@ mod tests {
#[test]
fn test_clone_to_foreign_string_cow() {
- let p = c"Hello, world!".as_ptr();
+ let p = cstr!("Hello, world!").as_ptr();
for s in vec![
Into::<Cow<str>>::into("Hello, world!"),
Into::<Cow<str>>::into("Hello, world!".to_string())] {
@@ -663,7 +665,7 @@ mod tests {
#[test]
fn test_clone_to_foreign_string() {
let s = "Hello, world!".to_string();
- let cstr = c"Hello, world!";
+ let cstr = cstr!("Hello, world!");
let cloned = s.clone_to_foreign();
unsafe {
let len = libc::strlen(cloned.as_ptr());
@@ -683,7 +685,7 @@ mod tests {
fn test_option() {
// An Option can be used to produce or convert NULL pointers
let s = Some("Hello, world!".to_string());
- let cstr = c"Hello, world!";
+ let cstr = cstr!("Hello, world!");
unsafe {
assert_eq!(Option::<String>::cloned_from_foreign(cstr.as_ptr()), s);
assert_matches!(Option::<String>::cloned_from_foreign(ptr::null()), None);
@@ -695,7 +697,7 @@ mod tests {
fn test_box() {
// A box can be produced if the inner type has the capability.
let s = Box::new("Hello, world!".to_string());
- let cstr = c"Hello, world!";
+ let cstr = cstr!("Hello, world!");
let cloned = unsafe { Box::<String>::cloned_from_foreign(cstr.as_ptr()) };
assert_eq!(s, cloned);
diff --git a/qemu/tests/main.rs b/qemu/tests/main.rs
index e499c14..601e92b 100644
--- a/qemu/tests/main.rs
+++ b/qemu/tests/main.rs
@@ -1,4 +1,5 @@
use const_default::ConstDefault;
+use cstr::cstr;
use qemu::qom_define_type;
use qemu::Object;
@@ -27,7 +28,7 @@ struct TestState {
}
qom_define_type!(
- c"test-object",
+ cstr!("test-object"),
TestObject,
TestConf,
();
@@ -37,12 +38,12 @@ qom_define_type!(
impl ObjectImpl for TestObject {}
qdev_define_type!(
- c"test-device",
+ cstr!("test-device"),
TestDevice,
TestConf,
RefCell<TestState>;
@extends DeviceState;
- @properties [qdev_prop!(bool, c"foo", TestDevice, true, foo)]
+ @properties [qdev_prop!(bool, cstr!("foo"), TestDevice, true, foo)]
);
impl TestDevice {
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 13/14] rust: introduce alternative to offset_of!
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (11 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 12/14] rust: replace c"" literals with cstr crate Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 14/14] rust: use version of toml_edit that does not require new Rust Paolo Bonzini
2024-07-04 19:26 ` [PATCH 00/14] rust: example of bindings code for Rust in QEMU Pierrick Bouvier
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Allow working with Rust versions prior to 1.77. The code was
taken from Rust's Discourse platform and is used with permission of
the author.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/Cargo.toml | 3 +
qemu/build.rs | 5 ++
qemu/src/hw/core/device_impl.rs | 4 +-
qemu/src/lib.rs | 4 ++
qemu/src/qom/object_impl.rs | 13 +++--
qemu/src/util/mod.rs | 1 +
qemu/src/util/offset_of.rs | 99 +++++++++++++++++++++++++++++++++
qemu/tests/main.rs | 11 +++-
9 files changed, 137 insertions(+), 10 deletions(-)
create mode 100644 qemu/build.rs
create mode 100644 qemu/src/util/offset_of.rs
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index a07a449..93808a5 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -12,3 +12,6 @@ cstr = { version = "=0.2.10" }
[dev-dependencies]
matches = ">=0"
+
+[build-dependencies]
+version_check = { version = "~0.9" }
diff --git a/qemu/build.rs b/qemu/build.rs
new file mode 100644
index 0000000..34f7b49
--- /dev/null
+++ b/qemu/build.rs
@@ -0,0 +1,5 @@
+fn main() {
+ if let Some(true) = version_check::is_min_version("1.77.0") {
+ println!("cargo:rustc-cfg=has_offset_of");
+ }
+}
diff --git a/qemu/src/hw/core/device_impl.rs b/qemu/src/hw/core/device_impl.rs
index 80b0e5e..b1d2f04 100644
--- a/qemu/src/hw/core/device_impl.rs
+++ b/qemu/src/hw/core/device_impl.rs
@@ -111,7 +111,7 @@ macro_rules! qdev_prop {
$kind,
$name,
(<$crate::conf_type!($type) as ConstDefault>::DEFAULT).$field,
- <$type as $crate::DeviceTypeImpl>::CONF_OFFSET + std::mem::offset_of!($crate::conf_type!($type), $field)
+ <$type as $crate::DeviceTypeImpl>::CONF_OFFSET + $crate::offset_of!($crate::conf_type!($type), $field)
)
};
}
@@ -126,7 +126,7 @@ macro_rules! qdev_define_type {
@extends $super $(,$supers)*, $crate::Object);
unsafe impl $crate::DeviceTypeImpl for $struct {
- const CONF_OFFSET: usize = std::mem::offset_of!($struct, conf);
+ const CONF_OFFSET: usize = $crate::offset_of!($struct, conf);
fn properties() -> *const $crate::Property {
static mut PROPERTIES: &'static [$crate::Property] = &[$($props),+];
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 3f0491c..2d43a25 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -31,3 +31,7 @@ pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
pub use util::zeroed::Zeroed;
pub type Result<T> = std::result::Result<T, Error>;
+
+// with_offsets is exported directly from util::offset_of
+#[cfg(has_offset_of)]
+pub use std::mem::offset_of;
diff --git a/qemu/src/qom/object_impl.rs b/qemu/src/qom/object_impl.rs
index 61546b6..b1768b9 100644
--- a/qemu/src/qom/object_impl.rs
+++ b/qemu/src/qom/object_impl.rs
@@ -95,11 +95,14 @@ unsafe fn rust_type_register<T: TypeImpl + ObjectImpl>() {
#[macro_export]
macro_rules! qom_define_type {
($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty; @extends $super:ty $(,$supers:ty)*) => {
- struct $struct {
- // self.base dropped by call to superclass instance_finalize
- base: std::mem::ManuallyDrop<$super>,
- conf: $conf_ty,
- state: $state_ty,
+ $crate::with_offsets! {
+ #[repr(C)]
+ struct $struct {
+ // self.base dropped by call to superclass instance_finalize
+ base: std::mem::ManuallyDrop<$super>,
+ conf: $conf_ty,
+ state: $state_ty,
+ }
}
// Define IsA markers for the struct itself and all the superclasses
diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
index 9c081b6..e4df7c9 100644
--- a/qemu/src/util/mod.rs
+++ b/qemu/src/util/mod.rs
@@ -1,3 +1,4 @@
pub mod error;
pub mod foreign;
+pub mod offset_of;
pub mod zeroed;
diff --git a/qemu/src/util/offset_of.rs b/qemu/src/util/offset_of.rs
new file mode 100644
index 0000000..4ce5188
--- /dev/null
+++ b/qemu/src/util/offset_of.rs
@@ -0,0 +1,99 @@
+#[cfg(not(has_offset_of))]
+#[macro_export]
+macro_rules! offset_of {
+ ($Container:ty, $field:ident) => {
+ <$Container>::offset_to.$field
+ };
+}
+
+/// A wrapper for struct declarations, that allows using `offset_of!` in
+/// versions of Rust prior to 1.77
+#[macro_export]
+macro_rules! with_offsets {
+ // source: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=10a22a9b8393abd7b541d8fc844bc0df
+ // used under MIT license with permission of Yandros aka Daniel Henry-Mantilla
+ (
+ #[repr(C)]
+ $(#[$struct_meta:meta])*
+ $struct_vis:vis
+ struct $StructName:ident {
+ $(
+ $(#[$field_meta:meta])*
+ $field_vis:vis
+ $field_name:ident : $field_ty:ty
+ ),*
+ $(,)?
+ }
+ ) => (
+ #[repr(C)]
+ $(#[$struct_meta])*
+ $struct_vis
+ struct $StructName {
+ $(
+ $(#[$field_meta])*
+ $field_vis
+ $field_name : $field_ty ,
+ )*
+ }
+
+ #[cfg(not(has_offset_of))]
+ #[allow(nonstandard_style)]
+ const _: () = {
+ pub
+ struct StructOffsets {
+ $(
+ $field_vis
+ $field_name: usize,
+ )*
+ }
+ struct Helper;
+ impl $StructName {
+ pub
+ const offset_to: StructOffsets = StructOffsets {
+ $(
+ $field_name: Helper::$field_name,
+ )*
+ };
+ }
+ const END_OF_PREV_FIELD: usize = 0;
+ $crate::with_offsets! {
+ @names [ $($field_name)* ]
+ @tys [ $($field_ty ,)*]
+ }
+ };
+ );
+
+ (
+ @names []
+ @tys []
+ ) => ();
+
+ (
+ @names [$field_name:ident $($other_names:tt)*]
+ @tys [$field_ty:ty , $($other_tys:tt)*]
+ ) => (
+ impl Helper {
+ const $field_name: usize = {
+ let align =
+ std::mem::align_of::<$field_ty>()
+ ;
+ let trail =
+ END_OF_PREV_FIELD % align
+ ;
+ 0 + END_OF_PREV_FIELD
+ + (align - trail)
+ * [1, 0][(trail == 0) as usize]
+ };
+ }
+ const _: () = {
+ const END_OF_PREV_FIELD: usize =
+ Helper::$field_name +
+ std::mem::size_of::<$field_ty>()
+ ;
+ $crate::with_offsets! {
+ @names [$($other_names)*]
+ @tys [$($other_tys)*]
+ }
+ };
+ );
+}
diff --git a/qemu/tests/main.rs b/qemu/tests/main.rs
index 601e92b..854c626 100644
--- a/qemu/tests/main.rs
+++ b/qemu/tests/main.rs
@@ -14,11 +14,16 @@ use qemu::DeviceState;
use qemu::Result;
+use qemu::with_offsets;
+
use std::cell::RefCell;
-#[derive(Default, ConstDefault)]
-struct TestConf {
- foo: bool,
+with_offsets! {
+ #[repr(C)]
+ #[derive(Default, ConstDefault)]
+ struct TestConf {
+ foo: bool,
+ }
}
#[derive(Default)]
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 14/14] rust: use version of toml_edit that does not require new Rust
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (12 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 13/14] rust: introduce alternative to offset_of! Paolo Bonzini
@ 2024-07-01 14:58 ` Paolo Bonzini
2024-07-04 19:26 ` [PATCH 00/14] rust: example of bindings code for Rust in QEMU Pierrick Bouvier
14 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-01 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
toml_edit is quite aggressive in bumping the minimum required
version of Rust. Force usage of an old version that runs
with 1.63.0.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu/Cargo.toml | 3 +++
2 files changed, 71 insertions(+), 3 deletions(-)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 93808a5..3ce5dba 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -15,3 +15,6 @@ matches = ">=0"
[build-dependencies]
version_check = { version = "~0.9" }
+
+# pick older version in order to support Rust 1.63
+toml_edit = { version = "~0.14" }
--
2.45.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
@ 2024-07-03 12:48 ` Marc-André Lureau
2024-07-03 12:58 ` Marc-André Lureau
2024-07-03 15:55 ` Paolo Bonzini
2024-09-27 16:09 ` Kevin Wolf
2024-09-27 19:36 ` Stefan Hajnoczi
2 siblings, 2 replies; 27+ messages in thread
From: Marc-André Lureau @ 2024-07-03 12:48 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé, Hanna Czenczek,
Stefan Hajnoczi, Sebastian Dröge
[-- Attachment #1: Type: text/plain, Size: 12020 bytes --]
Hi
(adding Sebastian, one of the glib-rs developers in CC)
On Mon, Jul 1, 2024 at 7:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> The qemu::util::foreign module provides:
>
> - A trait for structs that can be converted to a C ("foreign")
> representation
> (CloneToForeign)
>
> - A trait for structs that can be built from a C ("foreign") representation
> (FromForeign), and the utility IntoNative that can be used with less
> typing
> (similar to the standard library's From and Into pair)
>
> - Automatic implementations of the above traits for Option<>, supporting
> NULL
> pointers
>
> - A wrapper for a pointer that automatically frees the contained data. If
> a struct XYZ implements CloneToForeign, you can build an
> OwnedPointer<XYZ>
> and it will free the contents automatically unless you retrieve it with
> owned_ptr.into_inner()
>
You worry about technical debt, and I do too. Here you introduce quite
different traits than what glib-rs offers. We already touched this subject
2y ago, my opinion didn't change much (
https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-13-marcandre.lureau@redhat.com/).
Also, you don't offer the equivalent of "to_glib_none" which uses a
temporary stash and is quite useful, as a majority of functions don't take
ownership.
Because much of our code is using GLib types and API style, I think we
should strive for something that is close (if not just the same) to what
glib-rs offers. It's already hard enough to handle one binding concept,
having 2 will only make the matter worse. Consider a type like
GHashTable<GUuid, QOM>, it will be very annoying to deal with if we have
different bindings traits and implementations and we will likely end up
duplicating glib-rs effort.
As for naming & consistency, glib-rs settled on something clearer imho:
from_glib_full
from_glib_none
to_glib_full
to_glib_none
vs
from_foreign
cloned_from_foreign
clone_to_foreign
/nothing/
but overall, this is still close enough that we shouldn't reinvent it.
It may be worth studying what the kernel offers, I haven't checked yet.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu/src/lib.rs | 6 +
> qemu/src/util/foreign.rs | 247 +++++++++++++++++++++++++++++++++++++++
> qemu/src/util/mod.rs | 1 +
> 3 files changed, 254 insertions(+)
> create mode 100644 qemu/src/util/foreign.rs
> create mode 100644 qemu/src/util/mod.rs
>
> diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
> index fce21d7..c48edcf 100644
> --- a/qemu/src/lib.rs
> +++ b/qemu/src/lib.rs
> @@ -2,3 +2,9 @@
> #![allow(dead_code)]
>
> pub mod bindings;
> +
> +pub mod util;
> +pub use util::foreign::CloneToForeign;
> +pub use util::foreign::FromForeign;
> +pub use util::foreign::IntoNative;
> +pub use util::foreign::OwnedPointer;
> diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
> new file mode 100644
> index 0000000..a591925
> --- /dev/null
> +++ b/qemu/src/util/foreign.rs
> @@ -0,0 +1,247 @@
> +// TODO: change to use .cast() etc.
> +#![allow(clippy::ptr_as_ptr)]
> +
> +/// Traits to map between C structs and native Rust types.
> +/// Similar to glib-rs but a bit simpler and possibly more
> +/// idiomatic.
> +use std::borrow::Cow;
> +use std::fmt;
> +use std::fmt::Debug;
> +use std::mem;
> +use std::ptr;
> +
> +/// A type for which there is a canonical representation as a C datum.
> +pub trait CloneToForeign {
> + /// The representation of `Self` as a C datum. Typically a
> + /// `struct`, though there are exceptions for example `c_char`
> + /// for strings, since C strings are of `char *` type).
> + type Foreign;
> +
> + /// Free the C datum pointed to by `p`.
> + ///
> + /// # Safety
> + ///
> + /// `p` must be `NULL` or point to valid data.
> + unsafe fn free_foreign(p: *mut Self::Foreign);
> +
> + /// Convert a native Rust object to a foreign C struct, copying
> + /// everything pointed to by `self` (same as `to_glib_full` in
> `glib-rs`)
> + fn clone_to_foreign(&self) -> OwnedPointer<Self>;
> +
> + /// Convert a native Rust object to a foreign C pointer, copying
> + /// everything pointed to by `self`. The returned pointer must
> + /// be freed with the `free_foreign` associated function.
> + fn clone_to_foreign_ptr(&self) -> *mut Self::Foreign {
> + self.clone_to_foreign().into_inner()
> + }
> +}
> +
> +impl<T> CloneToForeign for Option<T>
> +where
> + T: CloneToForeign,
> +{
> + type Foreign = <T as CloneToForeign>::Foreign;
> +
> + unsafe fn free_foreign(x: *mut Self::Foreign) {
> + T::free_foreign(x)
> + }
> +
> + fn clone_to_foreign(&self) -> OwnedPointer<Self> {
> + // Same as the underlying implementation, but also convert `None`
> + // to a `NULL` pointer.
> + self.as_ref()
> + .map(CloneToForeign::clone_to_foreign)
> + .map(OwnedPointer::into)
> + .unwrap_or_default()
> + }
> +}
> +
> +impl<T> FromForeign for Option<T>
> +where
> + T: FromForeign,
> +{
> + unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
> + // Same as the underlying implementation, but also accept a
> `NULL` pointer.
> + if p.is_null() {
> + None
> + } else {
> + Some(T::cloned_from_foreign(p))
> + }
> + }
> +}
> +
> +impl<T> CloneToForeign for Box<T>
> +where
> + T: CloneToForeign,
> +{
> + type Foreign = <T as CloneToForeign>::Foreign;
> +
> + unsafe fn free_foreign(x: *mut Self::Foreign) {
> + T::free_foreign(x)
> + }
> +
> + fn clone_to_foreign(&self) -> OwnedPointer<Self> {
> + self.as_ref().clone_to_foreign().into()
> + }
> +}
> +
> +impl<T> FromForeign for Box<T>
> +where
> + T: FromForeign,
> +{
> + unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
> + Box::new(T::cloned_from_foreign(p))
> + }
> +}
> +
> +impl<'a, B> CloneToForeign for Cow<'a, B>
> + where B: 'a + ToOwned + ?Sized + CloneToForeign,
> +{
> + type Foreign = B::Foreign;
> +
> + unsafe fn free_foreign(ptr: *mut B::Foreign) {
> + B::free_foreign(ptr);
> + }
> +
> + fn clone_to_foreign(&self) -> OwnedPointer<Self> {
> + self.as_ref().clone_to_foreign().into()
> + }
> +}
> +
> +/// Convert a C datum into a native Rust object, taking ownership of
> +/// the C datum. You should not need to implement this trait
> +/// as long as Rust types implement `FromForeign`.
> +pub trait IntoNative<T> {
> + /// Convert a C datum to a native Rust object, taking ownership of
> + /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
> + ///
> + /// # Safety
> + ///
> + /// `p` must point to valid data, or can be `NULL` if Self is an
> + /// `Option` type. It becomes invalid after the function returns.
> + unsafe fn into_native(self) -> T;
> +}
> +
> +impl<T, U> IntoNative<U> for *mut T
> +where
> + U: FromForeign<Foreign = T>,
> +{
> + unsafe fn into_native(self) -> U {
> + U::from_foreign(self)
> + }
> +}
> +
> +/// A type which can be constructed from a canonical representation as a
> +/// C datum.
> +pub trait FromForeign: CloneToForeign + Sized {
> + /// Convert a C datum to a native Rust object, copying everything
> + /// pointed to by `p` (same as `from_glib_none` in `glib-rs`)
> + ///
> + /// # Safety
> + ///
> + /// `p` must point to valid data, or can be `NULL` is `Self` is an
> + /// `Option` type.
> + unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self;
> +
> + /// Convert a C datum to a native Rust object, taking ownership of
> + /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
> + ///
> + /// The default implementation calls `cloned_from_foreign` and frees
> `p`.
> + ///
> + /// # Safety
> + ///
> + /// `p` must point to valid data, or can be `NULL` is `Self` is an
> + /// `Option` type. `p` becomes invalid after the function returns.
> + unsafe fn from_foreign(p: *mut Self::Foreign) -> Self {
> + let result = Self::cloned_from_foreign(p);
> + Self::free_foreign(p);
> + result
> + }
> +}
> +
> +pub struct OwnedPointer<T: CloneToForeign + ?Sized> {
> + ptr: *mut <T as CloneToForeign>::Foreign,
> +}
> +
> +impl<T: CloneToForeign + ?Sized> OwnedPointer<T> {
> + /// Return a new `OwnedPointer` that wraps the pointer `ptr`.
> + ///
> + /// # Safety
> + ///
> + /// The pointer must be valid and live until the returned
> `OwnedPointer`
> + /// is dropped.
> + pub unsafe fn new(ptr: *mut <T as CloneToForeign>::Foreign) -> Self {
> + OwnedPointer { ptr }
> + }
> +
> + /// Safely create an `OwnedPointer` from one that has the same
> + /// freeing function.
> + pub fn from<U>(x: OwnedPointer<U>) -> Self
> + where
> + U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign> +
> ?Sized,
> + {
> + unsafe {
> + // SAFETY: the pointer type and free function are the same,
> + // only the type changes
> + OwnedPointer::new(x.into_inner())
> + }
> + }
> +
> + /// Safely convert an `OwnedPointer` into one that has the same
> + /// freeing function.
> + pub fn into<U>(self) -> OwnedPointer<U>
> + where
> + U: CloneToForeign<Foreign = <T as CloneToForeign>::Foreign>,
> + {
> + OwnedPointer::from(self)
> + }
> +
> + /// Return the pointer that is stored in the `OwnedPointer`. The
> + /// pointer is valid for as long as the `OwnedPointer` itself.
> + pub fn as_ptr(&self) -> *const <T as CloneToForeign>::Foreign {
> + self.ptr
> + }
> +
> + pub fn as_mut_ptr(&self) -> *mut <T as CloneToForeign>::Foreign {
> + self.ptr
> + }
> +
> + /// Return the pointer that is stored in the `OwnedPointer`,
> + /// consuming the `OwnedPointer` but not freeing the pointer.
> + pub fn into_inner(mut self) -> *mut <T as CloneToForeign>::Foreign {
> + let result = mem::replace(&mut self.ptr, ptr::null_mut());
> + mem::forget(self);
> + result
> + }
> +}
> +
> +impl<T: FromForeign + ?Sized> OwnedPointer<T> {
> + /// Convert a C datum to a native Rust object, taking ownership of
> + /// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
> + pub fn into_native(self) -> T {
> + // SAFETY: the pointer was passed to the unsafe constructor
> OwnedPointer::new
> + unsafe { T::from_foreign(self.into_inner()) }
> + }
> +}
> +
> +impl<T: CloneToForeign + ?Sized> Debug for OwnedPointer<T> {
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + let name = std::any::type_name::<T>();
> + let name = format!("OwnedPointer<{}>", name);
> + f.debug_tuple(&name).field(&self.as_ptr()).finish()
> + }
> +}
> +
> +impl<T: CloneToForeign + Default + ?Sized> Default for OwnedPointer<T> {
> + fn default() -> Self {
> + <T as Default>::default().clone_to_foreign()
> + }
> +}
> +
> +impl<T: CloneToForeign + ?Sized> Drop for OwnedPointer<T> {
> + fn drop(&mut self) {
> + let p = mem::replace(&mut self.ptr, ptr::null_mut());
> + // SAFETY: the pointer was passed to the unsafe constructor
> OwnedPointer::new
> + unsafe { T::free_foreign(p) }
> + }
> +}
> diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
> new file mode 100644
> index 0000000..be00d0c
> --- /dev/null
> +++ b/qemu/src/util/mod.rs
> @@ -0,0 +1 @@
> +pub mod foreign;
> --
> 2.45.2
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 15534 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-03 12:48 ` Marc-André Lureau
@ 2024-07-03 12:58 ` Marc-André Lureau
2024-07-03 15:55 ` Paolo Bonzini
1 sibling, 0 replies; 27+ messages in thread
From: Marc-André Lureau @ 2024-07-03 12:58 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé, Hanna Czenczek,
Stefan Hajnoczi, Sebastian Dröge
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
Hi
On Wed, Jul 3, 2024 at 4:48 PM Marc-André Lureau <marcandre.lureau@gmail.com>
wrote:
> Hi
>
> (adding Sebastian, one of the glib-rs developers in CC)
>
> On Mon, Jul 1, 2024 at 7:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> The qemu::util::foreign module provides:
>>
>> - A trait for structs that can be converted to a C ("foreign")
>> representation
>> (CloneToForeign)
>>
>> - A trait for structs that can be built from a C ("foreign")
>> representation
>> (FromForeign), and the utility IntoNative that can be used with less
>> typing
>> (similar to the standard library's From and Into pair)
>>
>> - Automatic implementations of the above traits for Option<>, supporting
>> NULL
>> pointers
>>
>> - A wrapper for a pointer that automatically frees the contained data. If
>> a struct XYZ implements CloneToForeign, you can build an
>> OwnedPointer<XYZ>
>> and it will free the contents automatically unless you retrieve it with
>> owned_ptr.into_inner()
>>
>
> [...] Also, you don't offer the equivalent of "to_glib_none" which uses a
> temporary stash and is quite useful, as a majority of functions don't take
> ownership.
>
I realize this may be somewhat offered by OwnedPointer (although not
mentioned explicitly in the doc). Its usage seems unsafe, as you have to
handle the foreign pointer lifetime manually...
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2274 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-03 12:48 ` Marc-André Lureau
2024-07-03 12:58 ` Marc-André Lureau
@ 2024-07-03 15:55 ` Paolo Bonzini
1 sibling, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-03 15:55 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé, Hanna Czenczek,
Stefan Hajnoczi, Sebastian Dröge
[warning: long email]
On Wed, Jul 3, 2024 at 2:48 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> (adding Sebastian, one of the glib-rs developers in CC)
>
> On Mon, Jul 1, 2024 at 7:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> The qemu::util::foreign module provides:
>>
>> - A trait for structs that can be converted to a C ("foreign") representation
>> - A trait for structs that can be built from a C ("foreign") representation
>> - A wrapper for a pointer that automatically frees the contained data.
>
> You worry about technical debt, and I do too. Here you introduce quite different traits than what glib-rs offers.
> We already touched this subject 2y ago, my opinion didn't change much
> (https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-13-marcandre.lureau@redhat.com/)
Hi Marc-André, first of all thanks for reviewing and, probably, sorry
for not going more into detail as to why I reinvented this particular
wheel.
Like two years ago, I find the full/none nomenclature very confusing
and there is no trace of them in QEMU, but I could get over that
easily.
But also, I think the two have different focus and design. All I
wanted was a place to put common code to convert Rust objects (mostly
CStr/String/str and Error) to and from C representations (char * and
QEMU's Error*), to avoid rewriting it over and over. So I wanted the
traits to be as simple as possible, and I didn't like that glib puts
to_glib_none and to_glib_full into the same trait, though I understand
why it does so. I also didn't like that to_glib_full() is an express
ticket to a memory leak. :)
OwnedPointer<> and free_foreign solve all the qualms I had with glib-rs:
- OwnedPointer<> takes care of freeing at the cost of extra verbosity
for ".as_ptr()".
- because clone_to_foreign() does not leak, I don't need to think
about to_glib_none() from the get go.
This has a ripple effect on other parts of the design. For example,
because glib has to_glib_none(), its basic design is to never allocate
malloc-ed memory unless ownership of the pointer is passed to C.
glib-rs's to_glib_none can and will "clone to Rust-managed memory" if
needed. Instead, because I have free_foreign(), I can malloc freely.
In the future we could add something like Stash/to_glib_none(), but it
would be very different and with performance properties enforced by
the type system. It would "create a pointer into *existing*
Rust-managed memory" cheaply but, every time a cheap borrow is not
possible, users will have to call clone_to_foreign() explicitly. In
other words there will be a difference between OwnedPointer<> as the
result of an expensive operation, and [the equivalent of glib-rs]
Stash as the result of a cheap operation. This is unlike
to_glib_none() which is e.g. cheap for CStr but not for String. More
on this below.
> Also, you don't offer the equivalent of "to_glib_none" which uses a temporary stash and is quite useful, as a majority of functions don't take ownership.
Right, in part you have already answered that: OwnedPointer handles
freeing in the same way as g_autofree, so you can use it as a
to_glib_none() replacement. If you need to pass the pointer and forget
about it, you can use clone_to_foreign_ptr(). If you need something
temporary that needs to be freed before returning, you use
clone_to_foreign() and as_ptr().
This is not any more unsafe than to_glib_none(). If you pass a
temporary pointer (x.clone_to_foreign().as_ptr()) where C code expects
full ownership, you get a dangling pointer either way. It is less
leak-happy than to_glib_full() though.
Regarding how to obtain a pointer cheaply, there are two use cases:
- borrow:
fn borrow_foreign<'a>(&'a self) -> Stash<'a,
<Self as CloneToForeign>::Foreign, &'a Self>;
Useful for slices and elementary integer types - including cases where
C parameters are passed by reference - and also for CStr. However
right now my main user is strings and it doesn't buy much there.
Because String and &str are not null-terminated, a "borrow" would
allocate memory, as in to_glib_none(), which is pretty weird. At this
point, I find it better to have the more explicit clone_to_foreign()
that tells you how expensive it is.
- consumption, which does not exist in glib-rs:
fn into_foreign(self) -> Stash<'static,
<Self as CloneToForeign>::Foreign, Self>;
This works for types that, like String, deref to a stable address even
when moved---this way we don't need to deal with pinning. And it is in
fact especially nice for the String case and therefore for anything
that uses format!(), because it could be implemented as
s.reserve_exact(1);
s.push('\0');
The two would be separate traits. There was one case in which I could
have used into_foreign(), namely the call to error_setg_internal() in
patch 5. But I am not sure if this will be more common than functions
taking &str, so I left it for later.
> Because much of our code is using GLib types and API style, I
> think we should strive for something that is close (if not just the
> same) to what glib-rs offers. It's already hard enough to handle
> one binding concept, having 2 will only make the matter worse.
> Consider a type like GHashTable<GUuid, QOM>, it will be very
> annoying to deal with if we have different bindings traits and
> implementations and we will likely end up duplicating glib-rs
> effort.
I'm not even sure if it's even true that much QEMU code is using GLib
types. We use quite a bit of GLib, and took a lot of inspiration from
it because GLib is good. But it's notable that we use very little GLib
*across modules*. There are only a handful of occurrences to
GList/GList/GHashTable in function arguments in include/, for all of
QEMU, and all of them are very rarely used functions. Why? Possibly
because, even where we take inspiration (the Error** idiom, QOM
casts), the implementation ends up completely different for reasons
that aren't just NIH.
In any case I don't see us using glib-rs much, if at all; uses of hash
tables in Rust code can just use HashMap<>. Yes, there is no
equivalent of the expensive, O(n) conversion
self.some_g_hash_table.to_glib_none(); but that's a feature, not a
limitation.
Also, anyway we couldn't extend the glib traits a lot, and glib-rs
only supports HashMap<String, String>.
> As for naming & consistency, glib-rs settled on something clearer imho:
>
> from_glib_full
> from_glib_none
> to_glib_full
> to_glib_none
>
> vs
>
> from_foreign
> cloned_from_foreign
> clone_to_foreign
> /nothing/
These last two are respectively clone_to_foreign_ptr and
clone_to_foreign. I am not wed to my names but I'm not sure the glib
ones are clearer.
What I want from names is clarity on who allocates or frees; I do that
with "clone", which is a common Rust term, to indicate that the C side
still needs to be freed after the call. I find it confusing that
from_glib_full() requires no freeing but to_glib_full() requires
freeing, for example - I understand _why_ but still "full" and "none"
are GIR terms that are not really familiar to most Rust or QEMU
developers.
And secondarily, clarity on what is cheap and what isn't. I don't have
that functionality at all for now, but the nomenclature is easily
extended to borrow_foreign() and into_foreign(). Talking about Rust
terms, glib-rs lacks "into" which is the standard Rust way to do
consuming conversions, while my code has into_native().
All in all, I think there are some significant differences between
glib-rs and this implementation that justify the effort.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 00/14] rust: example of bindings code for Rust in QEMU
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
` (13 preceding siblings ...)
2024-07-01 14:58 ` [PATCH 14/14] rust: use version of toml_edit that does not require new Rust Paolo Bonzini
@ 2024-07-04 19:26 ` Pierrick Bouvier
2024-07-05 8:06 ` Paolo Bonzini
14 siblings, 1 reply; 27+ messages in thread
From: Pierrick Bouvier @ 2024-07-04 19:26 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: Manos Pitsidianakis, Peter Maydell, Alex Bennée,
Daniel P . Berrangé, Marc-André Lureau, Hanna Czenczek,
Stefan Hajnoczi
Hi Paolo,
thanks for this series!
Some comments below.
On 7/1/24 07:58, Paolo Bonzini wrote:
> Hi all,
>
> this is an example of what some bindings code for QEMU would look like.
> Note that some parts of this code are barely expected to compile, and
> are probably full of bugs, but still should look like finished code
> (in fact, because they compile, the type system parts should be okay;
> though a conditional "should" is required).
>
> This code is not integrated in the QEMU source tree, because again it
> is just a example of what kind of Rust code would exist to handle the
> C<->Rust FFI. The translation of a handful of C structs and function
> prototypes is done by hand rather than with bindgen, in particular.
>
> The patches are organized as follows:
>
> Patches 1-2 introduce the skeleton for the rest of the code and are
> not particularly interesting, since that skeleton would be provided
> by the patches that introduce Rust usage in QEMU.
>
>
> Patches 3-4 define common code to handle conversion of data structures
> between Rust and C. I couldn't find an existing crate to do this,
> though there are similar concepts in glib-rs. The crate defines
> variants of Clone, From and Into that convert a Rust struct to a
> C pointer, for example:
>
> let s = "abc".clone_to_foreign(); // mallocs a NULL-terminated copy
> let p = s.as_ptr();
> drop(s); // p is freed now
>
> or the other way round:
>
> let s = String::cloned_from_foreign(p); // p is a *const c_char
>
> let t: String = p.into_native(); // p is a *mut c_char and is free()d
>
> The second patch defines what you need to convert strings from and to
> C. It's used by tests but also by a couple of QOM functions implemented
> below; it lets you forget the differences between String, &str, CString,
> &CStr and Cow<'_, str>.
>
> This is the only complete part of the skeleton (in fact I wrote it
> a couple years ago), and it comes with testcases that pass in both
> "#[test]" and "doctest" (i.e. snippets integrated in the documentation
> comments) styles.
>
>
> Patch 5 (mostly complete except for &error_abort support and missing
> tests) allows using the above functionality for the QEMU Error*.
>
>
> Patches 6-8 provide an example of how to use QOM from Rust. They
> define all the functionality to:
>
> - handle reference counting
>
> - handle typesafe casting (i.e. code doesn't compile if an upcast
> is invalid, or if a downcast cannot possibly succeed).
>
> - faking inheritance since Rust doesn't support it.
>
> While I started with some implementation ideas in glib-rs, this has
> evolved quite a bit and doesn't really look much like glib-rs anymore.
> I provided a handful of functions to wrap C APIs. For example:
>
> object_property_add_child(obj, "foo", object_new(TYPE_BLAH))
>
> becomes
>
> obj.property_add_child("foo", Blah::new());
>
> ... except that the former leaks a reference and the latter does not
> (gotcha :)). The idea is that these functions would be written, in
> general, as soon as Rust code uses them, but I included a few as an
> example of using the various abstractions for typecasting, reference
> counting, and converting from/to C data strcutures.
>
> A large part of this code is of the "it compiles and it looks nice, it
> must be perfect" kind. Rust is _very_ picky on type safety, in case you
> didn't know. One day we're all going to be programming in Haskell,
> without even noticing it.
>
>
> Patches 9-10 deal with how to define new subclasses in Rust. They are
> a lot less polished and less ready. There is probably a lot of polish
> that could be applied to make the code look nicer, but I guess there is
> always time to make the code look nicer until the details are sorted out.
>
> The things that I considered here are:
>
> - splitting configuration from runtime state. Configuration is
> immutable throughout the lifetime of the object, and holds the
> value of user-configured properties. State will typically be a
> Mutex<> or RefCell<> since the QOM bindings make wide use of interior
> mutability---almost all functions are declared as &self---following
> the lead of glib-rs.
>
> - automatic generation of instance_init and instance_finalize. For
> this I opted to introduce a new initialization step that is tailored to
> Rust, called instance_mem_init(), that is executed before the default
> value of properties is set. This makes sure that user code only ever
> sees valid values for the whole struct, including after a downcast;
> it matters because some Rust types (notably references) cannot be
> initialized to a zero-bytes pattern. The default implementation of
> instance_mem_init is simply a memset(), since the callback replaces the
>
> memset(obj, 0, type->instance_size);
>
> line in object_initialize_with_type(). I have prototyped this change
> in QEMU already.
>
> - generation of C vtables from safe code that is written in Rust. I
> chose a trait that only contains associated constants as a way to
> access the vtables generically. For example:
>
> impl ObjectImpl for TestDevice {
> const UNPARENT: Option<fn(&TestDevice)> = Some(TestDevice::unparent);
> }
>
> impl DeviceImpl for TestDevice {
> const REALIZE: Option<fn(&TestDevice) -> Result<()>> = Some(TestDevice::realize);
> }
>
> This works and it seems like a style that (in the future) we could apply
> macro or even procedural macro magic to.
>
> - generation of qdev property tables. While only boolean properties are
> implemented here, one idea that I experimented with, is that the
> default value of properties is derived from the ConstDefault trait.
> (ConstDefault is provided by the const_default external crate). Again,
> this is material for future conversion to procedural macros.
>
> I absolutely didn't look at vmstate, but it shouldn't be too different
> from properties, at least for the common cases.
>
>
> Patches 11-14 finally are an example of the changes that are needed
> to respect a minimum supported Rust version consistent with what is in
> Debian Bullseye. It's not too bad, especially since the current version
> of the QOM bindings does not require generic associated types anymore.
>
>
> Why am I posting this? Because this kind of glue code is the ultimate
> source of technical debt. It is the thing that we should be scared of
> when introducing a new language in QEMU. It makes it harder to change C
> code, and it is hard to change once Rust code becomes more widespread.
> If we think a C API is not fully baked, we probably shouldn't write
> Rust code that uses it (including bindings code). If we think a Rust
> API is not fully baked, we probably shouldn't add too much Rust code
> that uses it.
>
Shouldn't we focus more on how we want to write a QOM device in Rust
instead, by making abstraction of existing C implementation first?
Once we have satisfying idea, we could evaluate how it maps to existing
implementation, and which compromise should be made for efficiency.
It seems that the current approach is to stick to existing model, and
derive Rust bindings from this.
I agree this glue can be a source of technical debt, but on the other
side, it should be easy to refactor it, if we decided first what the
"clean and idiomatic" Rust API should look like.
A compromise where you have to manually translate some structs, or copy
memory to traverse languages borders at some point, could be worth the
safety and expressiveness.
> We should have an idea of what this glue code looks like, in order to make
> an informed choice. If we think we're not comfortable with reviewing it,
> well, we should be ready to say so and stick with C until we are.
>
While it is important that this glue code is maintainable and looks
great, I don't think it should be the main reason to accept usage of a
new language.
> The alternative could be to use Rust without this kind of binding. I
> think it's a bad idea. It removes many of the advantages of Rust
> (which are exemplified by the above object_property_add_child one-liner),
> and it also introduces _new_ kinds of memory errors, since Rust has
> its own undefined behavior conditions that are not there in C/C++.
> For example:
>
> impl Struct {
> pub fn f(&self) {
> call_some_c_function(Self::g, self as *const Self as *mut _);
> }
>
> fn do_g(&mut self) {
> ...
> }
>
> extern "C" fn g(ptr: *mut Self) {
> unsafe { &mut *ptr }.do_g();
> }
> }
>
> is invalid because a &mut reference (exclusive) is alive at the same time
> as a & reference (shared). It is left as an exercise to the reader to
> figure out all the possible ways in which we can shoot our own feet,
> considering the pervasive use of callbacks in QEMU.
>
I agree with this point that you made on the original RFC series from
Manos. Just calling qemu C API through unsafe code is not making the
best possible usage of Rust.
>
> With respect to callbacks, that's something that is missing in this
> prototype. Fortunately, that's also something that will be tackled very
> soon if the PL011 example is merged, because memory regions and character
> devices both introduce them. Also, as I understand it, Rust code using
> callbacks is not particularly nice anyway, though it is of course doable.
> Instead, this exercise are about being able to write *nice* Rust code,
> with all the advantages provided by the language, and the cost of
> writing/maintaining the glue code that makes it possible. I expect
> that we'll use a technique similar to the extern_c crate (it's 20 lines of
> code; https://docs.rs/crate/extern-c/) to convert something that implements
> Fn(), including a member function, into an extern "C" function.
>
It could be an interesting thing to explore. Is the best solution to use
specific traits (no callbacks involved from Rust code), or should we
have Rust closures that mimic this behavior.
We could have a specific trait with functions to handle various kind of
events. And the glue code could be responsible to translate this to
callbacks for the C side (and calling Rust code accordingly, eventually
serializing this on a single thread to avoid any race issues).
>
> Anyhow: I think we can do it, otherwise I would not have written 2000 lines
> of code (some of it two or three times). But if people are now scared and
> think we shouldn't, well, that's also a success of its own kind.
>
> Paolo
>
>
> Paolo Bonzini (14):
> add skeleton
> set expectations
> rust: define traits and pointer wrappers to convert from/to C
> representations
> rust: add tests for util::foreign
> rust: define wrappers for Error
> rust: define wrappers for basic QOM concepts
> rust: define wrappers for methods of the QOM Object class
> rust: define wrappers for methods of the QOM Device class
> rust: add idiomatic bindings to define Object subclasses
> rust: add idiomatic bindings to define Device subclasses
> rust: replace std::ffi::c_char with libc::c_char
> rust: replace c"" literals with cstr crate
> rust: introduce alternative to offset_of!
> rust: use version of toml_edit that does not require new Rust
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 00/14] rust: example of bindings code for Rust in QEMU
2024-07-04 19:26 ` [PATCH 00/14] rust: example of bindings code for Rust in QEMU Pierrick Bouvier
@ 2024-07-05 8:06 ` Paolo Bonzini
2024-07-05 18:52 ` Pierrick Bouvier
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-05 8:06 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Manos Pitsidianakis, Peter Maydell, Alex Bennée,
Daniel P . Berrangé, Marc-André Lureau, Hanna Czenczek,
Stefan Hajnoczi
On Thu, Jul 4, 2024 at 9:26 PM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
> > Patches 9-10 deal with how to define new subclasses in Rust. They are
> > a lot less polished and less ready. There is probably a lot of polish
> > that could be applied to make the code look nicer, but I guess there is
> > always time to make the code look nicer until the details are sorted out.
> >
> > The things that I considered here are:
> >
> > - splitting configuration from runtime state
> > - automatic generation of instance_init and instance_finalize
> > - generation of C vtables from safe code that is written in Rust
> > - generation of qdev property tables
>
> Shouldn't we focus more on how we want to write a QOM device in Rust
> instead, by making abstraction of existing C implementation first?
> Once we have satisfying idea, we could evaluate how it maps to existing
> implementation, and which compromise should be made for efficiency.
> It seems that the current approach is to stick to existing model, and
> derive Rust bindings from this.
I think I tried to do a little bit of that. For example, the split of
configuration from runtime state is done to isolate the interior
mutability, and the automatic generation of instance_init and
instance_finalize relies on Rust traits like Default and Drop; the
realize implementation returns a qemu::Result<()>; and so on.
But there are many steps towards converting the PL011 device to Rust:
- declarative definitions of bitfields and registers (done)
- using RefCell for mutable state
- using wrappers for class and property definition
- defining and using wrappers for Chardev/CharBackend
- defining and using wrappers for MemoryRegion callbacks
- defining and using wrappers for SysBusDevice functionality
- defining and using wrappers for VMState functionality
At each step we need to design "how we want to do that in Rust" and
all the things you mention above. This prototype covers the second and
third steps, and it's already big enough! :)
I expect that code like this one would be merged together with the
corresponding changes to PL011: the glue code is added to the qemu/
crate and used in the hw/core/pl011/ directory. This way, both authors
and reviewers will have a clue of what becomes more readable/idiomatic
in the resulting code.
> I agree this glue can be a source of technical debt, but on the other
> side, it should be easy to refactor it, if we decided first what the
> "clean and idiomatic" Rust API should look like.
That's true, especially if we want to add more macro magic on top. We
can decide later where to draw the line between complexity of glue
code and cleanliness of Rust code, and also move the line as we see
fit.
On the other hand, if we believe that _this_ code is already too much,
that's also a data point. Again: I don't think it is, but I want us to
make an informed decision.
> A compromise where you have to manually translate some structs, or copy
> memory to traverse languages borders at some point, could be worth the
> safety and expressiveness.
Yep, Error is an example of this. It's not the common case, so it's
totally fine to convert to and from C Error* (which also includes
copying the inner error string, from a String to malloc-ed char*) to
keep the APIs nicer.
> > We should have an idea of what this glue code looks like, in order to make
> > an informed choice. If we think we're not comfortable with reviewing it,
> > well, we should be ready to say so and stick with C until we are.
>
> While it is important that this glue code is maintainable and looks
> great, I don't think it should be the main reason to accept usage of a
> new language.
Not the main reason, but an important factor in judging if we are
*able* to accept usage of a new language. Maybe it's just a formal
step, but it feels safer to have _some_ code to show and to read, even
if it's just a prototype that barely compiles.
> We could have a specific trait with functions to handle various kind of
> events. And the glue code could be responsible to translate this to
> callbacks for the C side (and calling Rust code accordingly, eventually
> serializing this on a single thread to avoid any race issues).
That's a possibility, yes. Something like (totally random):
impl CharBackend {
pub fn start<T: CharBackendCallbacks>(&mut self, obj: &T) {
qemu_chr_fe_set_handlers(self.0,
Some(obj::can_receive),
Some(obj::receive, obj::event),
Some(obj::be_change), obj as *const _ as
*const c_void,
ptr::null(), true);
}
pub fn stop(&mut self) {
qemu_chr_fe_set_handlers(self.0, None, None,
None, ptr::null(), ptr::null(), true);
}
}
but I left for later because I focused just on the lowest levels code
where you could have "one" design. For example, for memory regions
some devices are going to have more than one, so there could be
reasons to do callbacks in different ways. By the way, all of this
passing of Rust references as opaque pointers needs, at least in
theory, to be annotated for pinning. Do we have to make sure that
pinning is handled correctly, or can we just assume that QOM objects
are pinned? I am not sure I am ready to answer the question...
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 00/14] rust: example of bindings code for Rust in QEMU
2024-07-05 8:06 ` Paolo Bonzini
@ 2024-07-05 18:52 ` Pierrick Bouvier
2024-07-05 21:08 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Pierrick Bouvier @ 2024-07-05 18:52 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Manos Pitsidianakis, Peter Maydell, Alex Bennée,
Daniel P . Berrangé, Marc-André Lureau, Hanna Czenczek,
Stefan Hajnoczi
On 7/5/24 01:06, Paolo Bonzini wrote:
> On Thu, Jul 4, 2024 at 9:26 PM Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>> Patches 9-10 deal with how to define new subclasses in Rust. They are
>>> a lot less polished and less ready. There is probably a lot of polish
>>> that could be applied to make the code look nicer, but I guess there is
>>> always time to make the code look nicer until the details are sorted out.
>>>
>>> The things that I considered here are:
>>>
>>> - splitting configuration from runtime state
>>> - automatic generation of instance_init and instance_finalize
>>> - generation of C vtables from safe code that is written in Rust
>>> - generation of qdev property tables
>>
>> Shouldn't we focus more on how we want to write a QOM device in Rust
>> instead, by making abstraction of existing C implementation first?
>> Once we have satisfying idea, we could evaluate how it maps to existing
>> implementation, and which compromise should be made for efficiency.
>> It seems that the current approach is to stick to existing model, and
>> derive Rust bindings from this.
>
> I think I tried to do a little bit of that. For example, the split of
> configuration from runtime state is done to isolate the interior
> mutability, and the automatic generation of instance_init and
> instance_finalize relies on Rust traits like Default and Drop; the
> realize implementation returns a qemu::Result<()>; and so on.
>
Patches 9/10 have a concrete example of what a TestDevice look like,
mixed with some glue definition. But definitely, this example code is
what would be the most interesting to review for introducing Rust.
To give an example of questions we could have:
Why should we have distinct structs for a device, the object
represented, and its state?
Properties could be returned by a specific function (properties()),
called at some given point. Would that be a good approach?
Having to implement ObjectImpl and DeviceImpl looks like an
implementation detail, that could be derived automatically from a
device. What's wrong with calling a realize that does nothing in the end?
Could device state be serialized with something like serde?
This is the kind of things that could be discussed, on a reduced
example, without specially looking at how to implement that concretely,
in a first time.
> But there are many steps towards converting the PL011 device to Rust:
>
> - declarative definitions of bitfields and registers (done)
> - using RefCell for mutable state
> - using wrappers for class and property definition
> - defining and using wrappers for Chardev/CharBackend
> - defining and using wrappers for MemoryRegion callbacks
> - defining and using wrappers for SysBusDevice functionality
> - defining and using wrappers for VMState functionality
>
> At each step we need to design "how we want to do that in Rust" and
> all the things you mention above. This prototype covers the second and
> third steps, and it's already big enough! :)
>
> I expect that code like this one would be merged together with the
> corresponding changes to PL011: the glue code is added to the qemu/
> crate and used in the hw/core/pl011/ directory. This way, both authors
> and reviewers will have a clue of what becomes more readable/idiomatic
> in the resulting code.
>
>> I agree this glue can be a source of technical debt, but on the other
>> side, it should be easy to refactor it, if we decided first what the
>> "clean and idiomatic" Rust API should look like.
>
> That's true, especially if we want to add more macro magic on top. We
> can decide later where to draw the line between complexity of glue
> code and cleanliness of Rust code, and also move the line as we see
> fit.
>
Automatic derivation macros could be ok (to derive some trait or
internal stuff), but usage of magic macros as syntactic sugar should
indeed be thought twice.
Return a collection of QDevProperty could be better than having a magic
@properties syntactic sugar.
Another thing that could be discussed is: do we want to have the whole
inheritance mechanism for Rust devices? Is it really needed?
> On the other hand, if we believe that _this_ code is already too much,
> that's also a data point. Again: I don't think it is, but I want us to
> make an informed decision.
>
Between having a clean and simple Device definition, with a bit more of
magic glue (hidden internally), and requires devices to do some magic
initialization to satisfy existing architecture, what would be the best?
Even though it takes 1000 more lines, I would be in favor to have
Devices that are as clean and simple as possible. Because this is the
kind of code what will be the most added/modified, compared to the glue
part.
>> A compromise where you have to manually translate some structs, or copy
>> memory to traverse languages borders at some point, could be worth the
>> safety and expressiveness.
>
> Yep, Error is an example of this. It's not the common case, so it's
> totally fine to convert to and from C Error* (which also includes
> copying the inner error string, from a String to malloc-ed char*) to
> keep the APIs nicer.
>
Yes, it's a good approach!
>>> We should have an idea of what this glue code looks like, in order to make
>>> an informed choice. If we think we're not comfortable with reviewing it,
>>> well, we should be ready to say so and stick with C until we are.
>>
>> While it is important that this glue code is maintainable and looks
>> great, I don't think it should be the main reason to accept usage of a
>> new language.
>
> Not the main reason, but an important factor in judging if we are
> *able* to accept usage of a new language. Maybe it's just a formal
> step, but it feels safer to have _some_ code to show and to read, even
> if it's just a prototype that barely compiles.
>
>> We could have a specific trait with functions to handle various kind of
>> events. And the glue code could be responsible to translate this to
>> callbacks for the C side (and calling Rust code accordingly, eventually
>> serializing this on a single thread to avoid any race issues).
>
> That's a possibility, yes. Something like (totally random):
>
> impl CharBackend {
> pub fn start<T: CharBackendCallbacks>(&mut self, obj: &T) {
> qemu_chr_fe_set_handlers(self.0,
> Some(obj::can_receive),
> Some(obj::receive, obj::event),
> Some(obj::be_change), obj as *const _ as
> *const c_void,
> ptr::null(), true);
> }
> pub fn stop(&mut self) {
> qemu_chr_fe_set_handlers(self.0, None, None,
> None, ptr::null(), ptr::null(), true);
> }
> }
Or have multiple traits matching every possible operation, and allow a
device to implement it or not, like Read/Write traits. And the glue code
could call qemu_chr_fe_set_handlers.
>
> but I left for later because I focused just on the lowest levels code
> where you could have "one" design. For example, for memory regions
> some devices are going to have more than one, so there could be
> reasons to do callbacks in different ways. By the way, all of this
> passing of Rust references as opaque pointers needs, at least in
> theory, to be annotated for pinning. Do we have to make sure that
> pinning is handled correctly, or can we just assume that QOM objects
> are pinned? I am not sure I am ready to answer the question...
>
I hope my answer helped to understand more my point that discussing the
interface is more important than discussing the glue needed.
> Paolo
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 00/14] rust: example of bindings code for Rust in QEMU
2024-07-05 18:52 ` Pierrick Bouvier
@ 2024-07-05 21:08 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-07-05 21:08 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Manos Pitsidianakis, Peter Maydell, Alex Bennée,
Daniel P . Berrangé, Marc-André Lureau, Hanna Czenczek,
Stefan Hajnoczi
Hi, first of all I want to clarify the raison d'etre for this posting,
which I have also explained to Manos. Nothing you see here is code
that will be certainly included in QEMU; it's (mostly) throwaway by
design. I don't have much attachment to any of the code except perhaps
the casting and reference counting stuff (which is in, like, the third
rewrite... I got nerd-sniped there :)). But at the same time I think
it's plausibly not too different (in look and complexity) from what
the actual code will look like.
It's also not an attempt to bypass/leapfrog other people in the
design; it's more a "let's be ready for this to happen" than a design
document. The first step and the more immediate focus remains the
build system integration.
But you have good questions and observations so I'll put something
below about the design as well.
On Fri, Jul 5, 2024 at 8:52 PM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
> To give an example of questions we could have:
>
> Why should we have distinct structs for a device, the object
> represented, and its state?
If you refer to the conf and state, a bit because of the different
traits required (ConstDefault is needed for properties but not the
rest) and especially because we can enforce immutability of
configuration vs. interior mutability of state.
In particular, from Manos's prototype I noticed that you need to
access the Chardev while the state is *not* borrowed; methods on the
Chardev call back into the device and the callback needs mutable
access to the state. So some kind of separation seems to be necessary,
for better or worse, unless interior mutability is achieved with a
dozen Cells (not great).
> Having to implement ObjectImpl and DeviceImpl looks like an
> implementation detail, that could be derived automatically from a
> device. What's wrong with calling a realize that does nothing in the end?
The problem is not calling a realize that does nothing, it's that you
are forced to override the superclass implementation (which might be
in C) if you don't go through Option<>. class_init methods update a
few superclass methods but not all of them.
The vtable in ObjectImpl/DeviceImpl could be generated via macros; for
now I did it by hand to avoid getting carried away too much with
syntactic sugar.
> Could device state be serialized with something like serde?
Probably not, there's extra complications for versioning. A long time
ago there was an attempt to have some kind of IDL embedded in C code
(not unlike Rust attributes) to automatically generate properties and
vmstate, but it was too ambitious.
(https://www.linux-kvm.org/images/b/b5/2012-forum-qidl-talk.pdf,
slides 1-9).
Generating property and vmstate declarations could be done with
"normal" macros just like in C, or with attribute macros (needs a lot
more code and experience, wouldn't do it right away). However, serde
for QAPI and/or visitors may be a possibility, after all JSON is
serde's bread and butter.
> This is the kind of things that could be discussed, on a reduced
> example, without specially looking at how to implement that concretely,
> in a first time.
There are some things that Rust really hates that you do, and they
aren't always obvious. Therefore in this exercise I tried to let
intuition guide me, and see how much the type system fought that
intuition (not much actually!). I started from the more technical and
less artistic part to see if I was able to get somewhere.
But yes, it's a great exercise to do this experiment from the opposite
end. Then the actual glue code will "meet in the middle", applying
lessons learnt from both experiments, with individual pieces of the
real interface implemented and applied to the PL011 sample at the same
time. The main missing thing in PL011 is DMA, otherwise it's a nice
playground.
> usage of magic macros as syntactic sugar should indeed be thought twice.
> Return a collection of QDevProperty could be better than having a magic
> @properties syntactic sugar.
No hard opinions there, sure. I went for the magic syntax because the
properties cannot be a const and I wanted to hide the ugly "static
mut", but certainly there could be better ways to do it.
> Another thing that could be discussed is: do we want to have the whole
> inheritance mechanism for Rust devices? Is it really needed?
Eh, this time unfortunately I think it is. Stuff like children and
properties, which are methods of Object, need to be available to
subclasses in instance_init and/or realize. Reset is in Device and we
have the whole Device/SysBusDevice/PCIDevice set of classes, so you
need to access superclass methods from subclasses. It's not a lot of
code though.
> Between having a clean and simple Device definition, with a bit more of
> magic glue (hidden internally), and requires devices to do some magic
> initialization to satisfy existing architecture, what would be the best?
> Even though it takes 1000 more lines, I would be in favor to have
> Devices that are as clean and simple as possible. Because this is the
> kind of code what will be the most added/modified, compared to the glue
> part.
That's an opinion I share but I wasn't sure it's universal, which was
another reason to prototype and post some "scary but perhaps
necessary" code.
> Or have multiple traits matching every possible operation, and allow a
> device to implement it or not, like Read/Write traits. And the glue code
> could call qemu_chr_fe_set_handlers.
Possibly, yes. https://lwn.net/Articles/863459/ you see many such
techniques: traits (gpio::Chip etc.), composition (device::Data),
lambdas (only for initialization).
The advantage of the lambda approach is that it scales to multiple
backends or multiple memory regions. We'll see.
> I hope my answer helped to understand more my point that discussing the
> interface is more important than discussing the glue needed.
Sure, and I don't think we are very much in disagreement, if at all.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
2024-07-03 12:48 ` Marc-André Lureau
@ 2024-09-27 16:09 ` Kevin Wolf
2024-09-27 20:19 ` Paolo Bonzini
2024-09-27 19:36 ` Stefan Hajnoczi
2 siblings, 1 reply; 27+ messages in thread
From: Kevin Wolf @ 2024-09-27 16:09 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
Hi Paolo,
as you asked me at KVM Forum to have a look at this, I'm going through
it now.
Am 01.07.2024 um 16:58 hat Paolo Bonzini geschrieben:
> The qemu::util::foreign module provides:
>
> - A trait for structs that can be converted to a C ("foreign") representation
> (CloneToForeign)
>
> - A trait for structs that can be built from a C ("foreign") representation
> (FromForeign), and the utility IntoNative that can be used with less typing
> (similar to the standard library's From and Into pair)
It makes sense to me that we'll need something to convert data and that
this usually means creating a new instance, i.e. cloning. However, while
it's obvious that this is similar to From/Into, the part I'm missing
here is what's different from it.
In other words, couldn't we just implement the normal From trait between
FFI types and the native equivalent?
> - Automatic implementations of the above traits for Option<>, supporting NULL
> pointers
This is nice.
> - A wrapper for a pointer that automatically frees the contained data. If
> a struct XYZ implements CloneToForeign, you can build an OwnedPointer<XYZ>
> and it will free the contents automatically unless you retrieve it with
> owned_ptr.into_inner()
Something here feels off to me.
At first, I thought it might be only about naming. This is not about
owning the pointer (which you probably do anyway), but that the pointer
owns the object it points to. This concept has in fact a name in Rust:
It's a Box.
The major difference compared to Box is that we're using a different
allocator. Not sure if the allocator APIs would actually be viable, but
they're not stable anyway - but let's at least name this thing in way
that draws the obvious parallel. Maybe ForeignBox.
But the other thing that doesn't feel quite right is how this is coupled
with CloneToForeign. Freeing is different from cloning, and it really
relates to the foreign type itself, and not to the one that can be
cloned into a foreign type.
Bringing both together, what a Box doesn't usually have is a function
pointer for freeing. We probably don't need it here either, almost
everything needs g_free(). There is a different free_foreign()
implementation for Error, but arguably this could be changed:
bindings::Error should then implement Drop for the inner value (with an
error_free_inner() which needs to be exported separately from C first),
and then ForeignBox can just drop the Error object and g_free() the
pointer itself like it would do with any other value.
(Your implementation actually calls free() instead of g_free(). We
generally try to avoid that in our C code, so we should probably avoid
it in Rust, too.)
Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
2024-07-03 12:48 ` Marc-André Lureau
2024-09-27 16:09 ` Kevin Wolf
@ 2024-09-27 19:36 ` Stefan Hajnoczi
2024-09-27 20:26 ` Paolo Bonzini
2 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2024-09-27 19:36 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
On Mon, 1 Jul 2024 at 11:02, Paolo Bonzini <pbonzini@redhat.com> wrote:
> +/// A type for which there is a canonical representation as a C datum.
> +pub trait CloneToForeign {
> + /// The representation of `Self` as a C datum. Typically a
> + /// `struct`, though there are exceptions for example `c_char`
> + /// for strings, since C strings are of `char *` type).
> + type Foreign;
> +
> + /// Free the C datum pointed to by `p`.
> + ///
> + /// # Safety
> + ///
> + /// `p` must be `NULL` or point to valid data.
> + unsafe fn free_foreign(p: *mut Self::Foreign);
> +
> + /// Convert a native Rust object to a foreign C struct, copying
> + /// everything pointed to by `self` (same as `to_glib_full` in `glib-rs`)
> + fn clone_to_foreign(&self) -> OwnedPointer<Self>;
I expected the return type to be OwnedPointer<Self::Foreign>. Is this a typo?
Also, why is the return type OwnedPointer<T> instead of just T? I
guess it's common to want a heap-allocated value here so you decided
to hard-code OwnedPointer<>, but I'm not sure.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-09-27 16:09 ` Kevin Wolf
@ 2024-09-27 20:19 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2024-09-27 20:19 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
On Fri, Sep 27, 2024 at 6:10 PM Kevin Wolf <kwolf@redhat.com> wrote:
> Am 01.07.2024 um 16:58 hat Paolo Bonzini geschrieben:
> > The qemu::util::foreign module provides:
> >
> > - A trait for structs that can be converted to a C ("foreign") representation
> > (CloneToForeign)
> >
> > - A trait for structs that can be built from a C ("foreign") representation
> > (FromForeign), and the utility IntoNative that can be used with less typing
> > (similar to the standard library's From and Into pair)
>
> It makes sense to me that we'll need something to convert data and that
> this usually means creating a new instance, i.e. cloning. However, while
> it's obvious that this is similar to From/Into, the part I'm missing
> here is what's different from it.
>
> In other words, couldn't we just implement the normal From trait between
> FFI types and the native equivalent?
In general yes. Using new traits has two advantages (all IMHO of
course). First, it makes it possible to tie the implementation to the
availability of a freeing function; second, it makes it possible to
define this always, whereas From is limited by the orphan rule (you
cannot provide implementations of a trait for a struct unless you are
the crate that defines either the struct or the trait).
> > - Automatic implementations of the above traits for Option<>, supporting NULL
> > pointers
>
> This is nice.
... for example, you can't have such a blanket implementation "impl<T,
U: From<T>> From<T> for Option<U> {}".
> > - A wrapper for a pointer that automatically frees the contained data. If
> > a struct XYZ implements CloneToForeign, you can build an OwnedPointer<XYZ>
> > and it will free the contents automatically unless you retrieve it with
> > owned_ptr.into_inner()
>
> Something here feels off to me.
>
> At first, I thought it might be only about naming. This is not about
> owning the pointer (which you probably do anyway), but that the pointer
> owns the object it points to. This concept has in fact a name in Rust:
> It's a Box.
>
> The major difference compared to Box is that we're using a different
> allocator. Not sure if the allocator APIs would actually be viable, but
> they're not stable anyway - but let's at least name this thing in way
> that draws the obvious parallel. Maybe ForeignBox.
> But the other thing that doesn't feel quite right is how this is coupled
> with CloneToForeign. Freeing is different from cloning, and it really
> relates to the foreign type itself, and not to the one that can be
> cloned into a foreign type.
I am not 100% convinced of the ForeignBox name but I see your point.
You'd prefer to have "impl FreeForeign for c_char" and "impl
CloneToForeign for str", where it's the CloneToForeign::Foreign
associated type (in this case c_char) that must implement FreeForeign.
Also, clone_to_foreign() for a str would return an
OwnedPointer<c_char>, not an OwnedPointer<str>. Never be too
optimistic about Rust, but that should be doable and I agree it is
better.
> Bringing both together, what a Box doesn't usually have is a function
> pointer for freeing. We probably don't need it here either, almost
> everything needs g_free().
Is that true? For example we could have qobject_unref, or qapi_free_*
as implementations of FreeForeign.
> There is a different free_foreign()
> implementation for Error, but arguably this could be changed:
> bindings::Error should then implement Drop for the inner value (with an
> error_free_inner() which needs to be exported separately from C first),
> and then ForeignBox can just drop the Error object and g_free() the
> pointer itself like it would do with any other value.
I think that's a bit too magical. Making Error always represent an
owned object seems a bit risky; instead I'm treating the bindings::XYZ
structs as very C-like objects, that are usually managed through
either OwnedPointer<...> (e.g. for a parameter) or *mut XYZ (for a
return value).
Thanks for looking at this. This kind of review is something that
we'll have to do a lot and it's better to have some early experience
of what they look like, independent of whether this code ultimately
will or will not be part of QEMU.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-09-27 19:36 ` Stefan Hajnoczi
@ 2024-09-27 20:26 ` Paolo Bonzini
2025-05-26 12:35 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2024-09-27 20:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek, Stefan Hajnoczi
On Fri, Sep 27, 2024 at 9:36 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Mon, 1 Jul 2024 at 11:02, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > +/// A type for which there is a canonical representation as a C datum.
> > +pub trait CloneToForeign {
> > + /// The representation of `Self` as a C datum. Typically a
> > + /// `struct`, though there are exceptions for example `c_char`
> > + /// for strings, since C strings are of `char *` type).
> > + type Foreign;
> > +
> > + /// Free the C datum pointed to by `p`.
> > + ///
> > + /// # Safety
> > + ///
> > + /// `p` must be `NULL` or point to valid data.
> > + unsafe fn free_foreign(p: *mut Self::Foreign);
> > +
> > + /// Convert a native Rust object to a foreign C struct, copying
> > + /// everything pointed to by `self` (same as `to_glib_full` in `glib-rs`)
> > + fn clone_to_foreign(&self) -> OwnedPointer<Self>;
>
> I expected the return type to be OwnedPointer<Self::Foreign>. Is this a typo?
Kevin noticed the same. I'd have to check if I am missing something
but it seems to be just tunnel vision.
> Also, why is the return type OwnedPointer<T> instead of just T? I
> guess it's common to want a heap-allocated value here so you decided
> to hard-code OwnedPointer<>, but I'm not sure.
Because at this point it looks like the most important conversion is
to have a clone (meaning its lifetime is independent of the copy) and
a value that is not movable (moves can be unpredictable and then usage
in C is messy). The main example is creating a QEMU Error from
something that implements the Rust std::error::Error trait.
Actually I have written extra code that _borrows_ into a foreign
object (so a CStr can be used as a *const c_char for as long as the
CStr is alive), but I didn't really have a user and wrote it only to
validate the concept.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
2024-09-27 20:26 ` Paolo Bonzini
@ 2025-05-26 12:35 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2025-05-26 12:35 UTC (permalink / raw)
To: Stefan Hajnoczi, Kevin Wolf
Cc: qemu-devel, Pierrick Bouvier, Manos Pitsidianakis, Peter Maydell,
Alex Bennée, Daniel P . Berrangé,
Marc-André Lureau, Hanna Czenczek
On 9/27/24 22:26, Paolo Bonzini wrote:
> On Fri, Sep 27, 2024 at 9:36 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>
>> On Mon, 1 Jul 2024 at 11:02, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> +/// A type for which there is a canonical representation as a C datum.
>>> +pub trait CloneToForeign {
>>> + /// The representation of `Self` as a C datum. Typically a
>>> + /// `struct`, though there are exceptions for example `c_char`
>>> + /// for strings, since C strings are of `char *` type).
>>> + type Foreign;
>>> +
>>> + /// Free the C datum pointed to by `p`.
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// `p` must be `NULL` or point to valid data.
>>> + unsafe fn free_foreign(p: *mut Self::Foreign);
>>> +
>>> + /// Convert a native Rust object to a foreign C struct, copying
>>> + /// everything pointed to by `self` (same as `to_glib_full` in `glib-rs`)
>>> + fn clone_to_foreign(&self) -> OwnedPointer<Self>;
>>
>> I expected the return type to be OwnedPointer<Self::Foreign>. Is this a typo?
>
> Kevin noticed the same. I'd have to check if I am missing something
> but it seems to be just tunnel vision.
It's been a while and probably everybody has forgotten about this.
Anyhow, the main reason why this returns for example
OwnedPointer<String> instead of OwnedPointer<c_char>, is that it tracks
whether the pointer can be NULL or not. A possibly-NULL pointer will be
OwnedPointer<Option<String>>; if the pointer is certainly non-NULL,
instead, it will be OwnedPointer<String>. So you can express
nullability in functions that accept an OwnedPointer<>.
Secondarily, it makes it possible to implement
OwnedPointer::into_native(). That is, it makes it easier to do the
opposite conversion once you're done with the foreign representation:
// s is a String
foreign_ptr = s.clone_to_foreign();
call_c_function(foreign_ptr);
*s = foreign_ptr.into_native();
as opposed to
foreign_ptr = s.clone_to_foreign();
call_c_function(foreign_ptr);
*s = String::from_foreign(foreign_ptr);
By the way, I have now extracted this library and published it on
crates.io with the name "foreign".
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-05-26 12:37 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-01 14:58 [PATCH 00/14] rust: example of bindings code for Rust in QEMU Paolo Bonzini
2024-07-01 14:58 ` [PATCH 01/14] add skeleton Paolo Bonzini
2024-07-01 14:58 ` [PATCH 02/14] set expectations Paolo Bonzini
2024-07-01 14:58 ` [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations Paolo Bonzini
2024-07-03 12:48 ` Marc-André Lureau
2024-07-03 12:58 ` Marc-André Lureau
2024-07-03 15:55 ` Paolo Bonzini
2024-09-27 16:09 ` Kevin Wolf
2024-09-27 20:19 ` Paolo Bonzini
2024-09-27 19:36 ` Stefan Hajnoczi
2024-09-27 20:26 ` Paolo Bonzini
2025-05-26 12:35 ` Paolo Bonzini
2024-07-01 14:58 ` [PATCH 04/14] rust: add tests for util::foreign Paolo Bonzini
2024-07-01 14:58 ` [PATCH 05/14] rust: define wrappers for Error Paolo Bonzini
2024-07-01 14:58 ` [PATCH 06/14] rust: define wrappers for basic QOM concepts Paolo Bonzini
2024-07-01 14:58 ` [PATCH 07/14] rust: define wrappers for methods of the QOM Object class Paolo Bonzini
2024-07-01 14:58 ` [PATCH 08/14] rust: define wrappers for methods of the QOM Device class Paolo Bonzini
2024-07-01 14:58 ` [PATCH 09/14] rust: add idiomatic bindings to define Object subclasses Paolo Bonzini
2024-07-01 14:58 ` [PATCH 10/14] rust: add idiomatic bindings to define Device subclasses Paolo Bonzini
2024-07-01 14:58 ` [PATCH 11/14] rust: replace std::ffi::c_char with libc::c_char Paolo Bonzini
2024-07-01 14:58 ` [PATCH 12/14] rust: replace c"" literals with cstr crate Paolo Bonzini
2024-07-01 14:58 ` [PATCH 13/14] rust: introduce alternative to offset_of! Paolo Bonzini
2024-07-01 14:58 ` [PATCH 14/14] rust: use version of toml_edit that does not require new Rust Paolo Bonzini
2024-07-04 19:26 ` [PATCH 00/14] rust: example of bindings code for Rust in QEMU Pierrick Bouvier
2024-07-05 8:06 ` Paolo Bonzini
2024-07-05 18:52 ` Pierrick Bouvier
2024-07-05 21:08 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).