[libvirt] [rust PATCH v2 1/5] libvirt-rust: stream: add more functions in stream

[Sahid Orentino Ferdjaoui]

On Tue, Dec 24, 2019 at 12:12:51AM -0700, Zixing Liu wrote:
> * added new functions: virStreamNew virStreamEventUpdateCallback
>   virStreamEventRemoveCallback
> * added new constants: VIR_STREAM_NONBLOCK
> * added new types/aliases: StreamFlags
> * changed the previous `new` associate function to `from_ptr` to avoid
>   naming conflicts
> * added basic tests to test the creation of Stream struct (can not test
>   the correctness of virStreamEventRemoveCallback and
>   virStreamEventUpdateCallback due to "unsupported by the connection
>   driver")
> 
> Signed-off-by: Zixing Liu <liushuyu at aosc.io>
> ---
>  src/stream.rs   | 42 +++++++++++++++++++++++++++++++++++++++++-
>  tests/stream.rs | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 1 deletion(-)
>  create mode 100644 tests/stream.rs

Reviewed-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui at canonical.com>

> diff --git a/src/stream.rs b/src/stream.rs
> index 8bcdf4b..de272ab 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,6 +18,8 @@
>  
>  extern crate libc;
>  
> +use connect::Connect;
> +use connect::sys::virConnectPtr;
>  use std::convert::TryFrom;
>  
>  use error::Error;
> @@ -31,6 +33,9 @@ pub mod sys {
>  
>  #[link(name = "virt")]
>  extern "C" {
> +    fn virStreamNew(c: virConnectPtr,
> +                    flags: libc::c_uint)
> +                     -> sys::virStreamPtr;
>      fn virStreamSend(c: sys::virStreamPtr,
>                       data: *const libc::c_char,
>                       nbytes: libc::size_t)
> @@ -42,6 +47,9 @@ extern "C" {
>      fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>      fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>      fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> +    fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> +                                    events: libc::c_int) -> libc::c_int;
> +    fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
>  }
>  
>  pub type StreamEventType = self::libc::c_uint;
> @@ -50,6 +58,9 @@ pub const VIR_STREAM_EVENT_WRITABLE: StreamEventType = (1 << 1);
>  pub const VIR_STREAM_EVENT_ERROR: StreamEventType = (1 << 2);
>  pub const VIR_STREAM_EVENT_HANGUP: StreamEventType = (1 << 3);
>  
> +pub type StreamFlags = self::libc::c_uint;
> +pub const VIR_STREAM_NONBLOCK: StreamFlags = (1 << 0);
> +
>  #[derive(Debug)]
>  pub struct Stream {
>      ptr: Option<sys::virStreamPtr>,
> @@ -68,7 +79,17 @@ impl Drop for Stream {
>  }
>  
>  impl Stream {
> -    pub fn new(ptr: sys::virStreamPtr) -> Stream {
> +    pub fn new(conn: &Connect, flags: StreamFlags) -> Result<Stream, Error> {
> +        unsafe {
> +            let ptr = virStreamNew(conn.as_ptr(), flags as libc::c_uint);
> +            if ptr.is_null() {
> +                return Err(Error::new());
> +            }

Is this block can be outside of the unsafe ? +1 if you add comment
explaining why it is safe to use conn.as_ptr().

> +            return Ok(Stream::from_ptr(ptr));
> +        }
> +    }
> +
> +    pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
>          Stream { ptr: Some(ptr) }
>      }
>  
> @@ -125,4 +146,23 @@ impl Stream {
>          };
>          usize::try_from(ret).map_err(|_| Error::new())
>      }
> +
> +    pub fn event_update_callback(&self, events: StreamEventType) -> Result<(), Error> {
> +        let ret = unsafe {
> +            virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int)

nit: Could be great to have a comment explaining why self.as_ptr() is safe to use.

> +        };
> +        if ret == -1 {
> +            return Err(Error::new());
> +        }
> +        return Ok(());
> +    }
> +
> +    pub fn event_remove_callback(&self) -> Result<(), Error> {
> +        unsafe {
> +            if virStreamEventRemoveCallback(self.as_ptr()) == -1 {

Same comment here.

> +                return Err(Error::new());
> +            }
> +            return Ok(());
> +        }
> +    }
>  }
> diff --git a/tests/stream.rs b/tests/stream.rs
> new file mode 100644
> index 0000000..16531b3
> --- /dev/null
> +++ b/tests/stream.rs
> @@ -0,0 +1,40 @@
> +/*
> + * 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.1 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, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Sahid Orentino Ferdjaoui <sahid.ferdjaoui at redhat.com>
> + */
> +
> +extern crate virt;
> +
> +mod common;
> +
> +use virt::stream::Stream;
> +use virt::stream::VIR_STREAM_NONBLOCK;
> +
> +#[test]
> +fn test_create_blocking() {
> +    let c = common::conn();
> +    let s = Stream::new(&c, 0).unwrap();
> +    drop(s);
> +    common::close(c);
> +}
> +
> +#[test]
> +fn test_create_non_blocking() {
> +    let c = common::conn();
> +    let s = Stream::new(&c, VIR_STREAM_NONBLOCK).unwrap();
> +    drop(s);
> +    common::close(c);
> +}
> -- 
> 2.24.1