diff mbox series

[v2,05/13] rust: remove uses of #[no_mangle]

Message ID 20241021163538.136941-6-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series rust: miscellaneous cleanups + QOM integration tests | expand

Commit Message

Paolo Bonzini Oct. 21, 2024, 4:35 p.m. UTC
Mangled symbols do not cause any issue; disabling mangling is only useful if
C headers reference the Rust function, which is not the case here.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       | 5 -----
 rust/hw/char/pl011/src/device_class.rs | 2 --
 rust/hw/char/pl011/src/memory_ops.rs   | 2 --
 rust/qemu-api/src/definitions.rs       | 1 -
 rust/qemu-api/src/device_class.rs      | 2 --
 5 files changed, 12 deletions(-)

Comments

Paolo Bonzini Oct. 23, 2024, 10:48 a.m. UTC | #1
On 10/21/24 18:35, Paolo Bonzini wrote:
> @@ -566,7 +563,6 @@ pub fn update(&self) {
>   /// # Safety
>   ///
>   /// We expect the FFI user of this function to pass a valid pointer for `chr`.
> -#[no_mangle]
>   pub unsafe extern "C" fn pl011_create(
>       addr: u64,
>       irq: qemu_irq,
This _needs_ to be no_mangle actually, because it is called from C.

Paolo
Zhao Liu Oct. 23, 2024, 2:06 p.m. UTC | #2
On Wed, Oct 23, 2024 at 12:48:33PM +0200, Paolo Bonzini wrote:
> Date: Wed, 23 Oct 2024 12:48:33 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH v2 05/13] rust: remove uses of #[no_mangle]
> 
> On 10/21/24 18:35, Paolo Bonzini wrote:
> > @@ -566,7 +563,6 @@ pub fn update(&self) {
> >   /// # Safety
> >   ///
> >   /// We expect the FFI user of this function to pass a valid pointer for `chr`.
> > -#[no_mangle]
> >   pub unsafe extern "C" fn pl011_create(
> >       addr: u64,
> >       irq: qemu_irq,
> This _needs_ to be no_mangle actually, because it is called from C.
>  

I'm a bit uncertain whether the "pub" keyword here should be removed.

I understand that pub and private are based on modules and do not apply
to FFI, but I haven't found any related documentation to confirm this.

However, technically, removing the pub here does not seem to cause any
issues during compilation and runtime.

Additionally, the pub keywords in the other functions involved in this
patch (except for the one in device_class_init) can also be removed, as
they are either only used as C callbacks or are indeed only used within
the current crate after macro expansion.

-Zhao
Zhao Liu Oct. 23, 2024, 2:13 p.m. UTC | #3
On Mon, Oct 21, 2024 at 06:35:30PM +0200, Paolo Bonzini wrote:
> Date: Mon, 21 Oct 2024 18:35:30 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH v2 05/13] rust: remove uses of #[no_mangle]
> X-Mailer: git-send-email 2.46.2
> 
> Mangled symbols do not cause any issue; disabling mangling is only useful if
> C headers reference the Rust function, which is not the case here.
> 
> Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs       | 5 -----
>  rust/hw/char/pl011/src/device_class.rs | 2 --
>  rust/hw/char/pl011/src/memory_ops.rs   | 2 --
>  rust/qemu-api/src/definitions.rs       | 1 -
>  rust/qemu-api/src/device_class.rs      | 2 --
>  5 files changed, 12 deletions(-)
> 

With pl011_create() fixed,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
diff mbox series

Patch

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index c7193b41bee..2b43f5e0939 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -514,7 +514,6 @@  pub fn update(&self) {
 /// We expect the FFI user of this function to pass a valid pointer, that has
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int {
     unsafe {
         debug_assert!(!opaque.is_null());
@@ -530,7 +529,6 @@  pub fn update(&self) {
 /// readable/writeable from one thread at any time.
 ///
 /// The buffer and size arguments must also be valid.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_receive(
     opaque: *mut core::ffi::c_void,
     buf: *const u8,
@@ -554,7 +552,6 @@  pub fn update(&self) {
 /// We expect the FFI user of this function to pass a valid pointer, that has
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) {
     unsafe {
         debug_assert!(!opaque.is_null());
@@ -566,7 +563,6 @@  pub fn update(&self) {
 /// # Safety
 ///
 /// We expect the FFI user of this function to pass a valid pointer for `chr`.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_create(
     addr: u64,
     irq: qemu_irq,
@@ -589,7 +585,6 @@  pub fn update(&self) {
 /// We expect the FFI user of this function to pass a valid pointer, that has
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
     unsafe {
         debug_assert!(!obj.is_null());
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index b7ab31af02d..2ad80451e87 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -46,7 +46,6 @@ 
 /// We expect the FFI user of this function to pass a valid pointer, that has
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) {
     unsafe {
         assert!(!dev.is_null());
@@ -60,7 +59,6 @@ 
 /// We expect the FFI user of this function to pass a valid pointer, that has
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
-#[no_mangle]
 pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) {
     unsafe {
         assert!(!dev.is_null());
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
index 8d066ebf6d0..5a5320e66c3 100644
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ b/rust/hw/char/pl011/src/memory_ops.rs
@@ -22,7 +22,6 @@ 
     },
 };
 
-#[no_mangle]
 unsafe extern "C" fn pl011_read(
     opaque: *mut core::ffi::c_void,
     addr: hwaddr,
@@ -44,7 +43,6 @@ 
     }
 }
 
-#[no_mangle]
 unsafe extern "C" fn pl011_write(
     opaque: *mut core::ffi::c_void,
     addr: hwaddr,
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 0b681c593f2..49ac59af123 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -53,7 +53,6 @@  extern "C" fn __load() {
         #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
         pub static LOAD_MODULE: extern "C" fn() = {
             extern "C" fn __load() {
-                #[no_mangle]
                 unsafe extern "C" fn $func() {
                     $body
                 }
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index b6b68cf9ce2..2219b9f73d0 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -9,7 +9,6 @@ 
 #[macro_export]
 macro_rules! device_class_init {
     ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => {
-        #[no_mangle]
         pub unsafe extern "C" fn $func(
             klass: *mut $crate::bindings::ObjectClass,
             _: *mut ::core::ffi::c_void,
@@ -103,7 +102,6 @@  const fn _calc_prop_len() -> usize {
             ]
         }
 
-        #[no_mangle]
         pub static mut $ident: $crate::device_class::Properties<PROP_LEN> = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties);
     };
 }