[Libguestfs] [PATCH] Rust bindings: Implement Event features
Hiroyuki Katsura
hiroyuki.katsura.0513 at gmail.com
Tue Jul 30 12:43:26 UTC 2019
> So this has a lifetime same as the whole handle?
Yes.
> Why do you then need to keep
> it in the list of callbacks then?
Because I want to make sure that callbacks must exist while they are
running.
> Also you explicitly leak it here, so even
> without a lifetime it will not get freed.
If you do not explicly write lifetime here, the pointer will have static
lifetime.
https://doc.rust-lang.org/std/boxed/struct.Box.html#method.leak
I think It is too long. But I think there is no more 'correct' lifetime
than 'a.
Actually, I wanted to use Weak::into_raw there. However, it is
nightly-feature now.
> if you do Box::into_raw instead of Box::leak, you get a *mut right away
> that you do not need to cast twice, I believe. Otherwise it should be
the same,
> I think.
If you use Box::into_raw, you must make sure that they are freed by
yourself.
However, if the box pointer is passed to the guestfs API, the pointer will
continue to live
without being freed after the guestfs handle which contains the passed
pointer has closed, I think.
On the other hand, by using Box::leak, you can assure that they live until
the handle closes.
But the buffer will be freed when the guestfs is dropped. So, passing this
pointer
will not cause memory leak.
2019年7月30日(火) 17:52 Martin Kletzander <mkletzan at redhat.com>:
> On Tue, Jul 30, 2019 at 02:51:37PM +0900, Hiroyuki Katsura wrote:
> >This patch includes:
> >
> >- Event callback handlers
> >- Tests related to events(410-430)
> >---
> > generator/rust.ml | 38 ++++++-
> > rust/src/base.rs | 24 +++--
> > rust/src/error.rs | 8 +-
> > rust/src/event.rs | 158 ++++++++++++++++++++++++++++
> > rust/src/lib.rs | 2 +
> > rust/tests/040_create_multiple.rs | 2 +-
> > rust/tests/410_close_event.rs | 38 +++++++
> > rust/tests/420_log_messages.rs | 60 +++++++++++
> > rust/tests/430_progress_messages.rs | 61 +++++++++++
> > 9 files changed, 381 insertions(+), 10 deletions(-)
> > create mode 100644 rust/src/event.rs
> > create mode 100644 rust/tests/410_close_event.rs
> > create mode 100644 rust/tests/420_log_messages.rs
> > create mode 100644 rust/tests/430_progress_messages.rs
> >
>
> [...]
>
> >diff --git a/rust/src/event.rs b/rust/src/event.rs
> >new file mode 100644
> >index 000000000..942feec69
> >--- /dev/null
> >+++ b/rust/src/event.rs
> >@@ -0,0 +1,158 @@
> >+/* libguestfs Rust bindings
> >+ * Copyright (C) 2019 Hiroyuki Katsura <hiroyuki.katsura.0513 at gmail.com>
> >+ *
> >+ * This library is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU Lesser General Public
> >+ * License as published by the Free Software Foundation; either
> >+ * version 2 of the License, or (at your option) any later version.
> >+ *
> >+ * This library is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * Lesser General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU Lesser General Public
> >+ * License along with this library; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >+ */
> >+
> >+use crate::base;
> >+use crate::error;
> >+use crate::guestfs;
> >+use std::ffi;
> >+use std::os::raw::{c_char, c_void};
> >+use std::slice;
> >+use std::sync;
> >+
> >+type GuestfsEventCallback = extern "C" fn(
> >+ *const base::guestfs_h,
> >+ *const c_void,
> >+ u64,
> >+ i32,
> >+ i32,
> >+ *const i8,
> >+ usize,
> >+ *const u64,
> >+ usize,
> >+);
> >+
> >+#[link(name = "guestfs")]
> >+extern "C" {
> >+ fn guestfs_set_event_callback(
> >+ g: *const base::guestfs_h,
> >+ cb: GuestfsEventCallback,
> >+ event_bitmask: u64,
> >+ flags: i32,
> >+ opaque: *const c_void,
> >+ ) -> i32;
> >+ fn guestfs_delete_event_callback(g: *const base::guestfs_h, eh: i32);
> >+ fn guestfs_event_to_string(bitmask: u64) -> *const c_char;
> >+ fn free(buf: *const c_void);
> >+}
> >+
> >+#[derive(Hash, PartialEq, Eq)]
> >+pub struct EventHandle {
> >+ eh: i32,
> >+}
> >+
> >+pub type Callback = FnMut(guestfs::Event, EventHandle, &[i8], &[u64]);
> >+
> >+fn events_to_bitmask(v: &[guestfs::Event]) -> u64 {
> >+ let mut r = 0u64;
> >+ for x in v.iter() {
> >+ r |= x.to_u64();
> >+ }
> >+ r
> >+}
> >+
> >+pub fn event_to_string(events: &[guestfs::Event]) -> Result<String,
> error::Error> {
> >+ let bitmask = events_to_bitmask(events);
> >+
> >+ let r = unsafe { guestfs_event_to_string(bitmask) };
> >+ if r.is_null() {
> >+ Err(error::unix_error("event_to_string"))
> >+ } else {
> >+ let s = unsafe { ffi::CStr::from_ptr(r) };
> >+ let s = s.to_str()?.to_string();
> >+ unsafe { free(r as *const c_void) };
> >+ Ok(s)
> >+ }
> >+}
> >+
> >+/* -- Why Not Box<Callback> but Arc<Callback> (in struct base::Handle)?
> --
> >+ * Assume that there are more than threads. While callback is running,
> >+ * if a thread frees the handle, automatically the buffer is freed if
> Box<Callback>
> >+ * is used. Therefore Arc<Callback> is used.
> >+ */
> >+
> >+impl<'a> base::Handle<'a> {
> >+ pub fn set_event_callback<C: 'a>(
> >+ &mut self,
> >+ callback: C,
> >+ events: &[guestfs::Event],
> >+ ) -> Result<EventHandle, error::Error>
> >+ where
> >+ C: Fn(guestfs::Event, EventHandle, &[u8], &[u64]),
> >+ {
> >+ extern "C" fn trampoline<C>(
> >+ _g: *const base::guestfs_h,
> >+ opaque: *const c_void,
> >+ event: u64,
> >+ event_handle: i32,
> >+ _flags: i32,
> >+ buf: *const c_char,
> >+ buf_len: usize,
> >+ array: *const u64,
> >+ array_len: usize,
> >+ ) where
> >+ C: Fn(guestfs::Event, EventHandle, &[u8], &[u64]),
> >+ {
> >+ // trampoline function
> >+ // c.f.
> https://s3.amazonaws.com/temp.michaelfbryan.com/callbacks/index.html
> >+
> >+ let event = match guestfs::Event::from_bitmask(event) {
> >+ Some(x) => x,
> >+ None => panic!("Failed to parse bitmask: {}", event),
> >+ };
> >+ let eh = EventHandle { eh: event_handle };
> >+ let buf = unsafe { slice::from_raw_parts(buf as *const u8,
> buf_len) };
> >+ let array = unsafe { slice::from_raw_parts(array, array_len)
> };
> >+
> >+ let callback_ptr = unsafe { &*(opaque as *const
> sync::Arc<C>) };
> >+ let callback = sync::Arc::clone(&callback_ptr);
> >+ callback(event, eh, buf, array)
> >+ }
> >+ let callback = sync::Arc::<C>::new(callback);
> >+ let event_bitmask = events_to_bitmask(events);
> >+
> >+ let eh = {
> >+ // Weak::into_raw is nightly.
> >+ // In order to make sure that callback is freed when handle
> is freed,
> >+ // lifetime is explicitly declared.
> >+ let ptr: &'a sync::Arc<C> =
> Box::leak(Box::new(callback.clone()));
>
> So this has a lifetime same as the whole handle? Why do you then need to
> keep
> it in the list of callbacks then? Also you explicitly leak it here, so
> even
> without a lifetime it will not get freed. Are you sure you wanted to do
> both?
>
> Also if you do Box::into_raw instead of Box::leak, you get a *mut right
> away
> that you do not need to cast twice, I believe. Otherwise it should be the
> same,
> I think.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190730/67154a95/attachment.htm>
More information about the Libguestfs
mailing list