[libvirt] [PATCH Rust 1/4] libvirt-rust: stream: add more functions in stream

[Sahid Orentino Ferdjaoui]

On Tue, Nov 12, 2019 at 02:44:42PM -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   | 43 +++++++++++++++++++++++++++++++++++++++++--
>  tests/stream.rs | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 2 deletions(-)
>  create mode 100644 tests/stream.rs
> 
> diff --git a/src/stream.rs b/src/stream.rs
> index 2ca5d0b..de272ab 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,8 +18,9 @@
>  
>  extern crate libc;
>  
> +use connect::Connect;
> +use connect::sys::virConnectPtr;
>  use std::convert::TryFrom;
> -use std::str;
>  
>  use error::Error;
>  
> @@ -32,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)
> @@ -43,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;
> @@ -51,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>,
> @@ -69,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 {

We probably just want to use unsafe when calling virStreamNew, and we
should try the most as possible to explain why it is safe to use
"unsafe". Basically here, we are safe because we specifically
allocated conn at Rust level.

> +            let ptr = virStreamNew(conn.as_ptr(), flags as libc::c_uint);
> +            if ptr.is_null() {
> +                return Err(Error::new());
> +            }
> +            return Ok(Stream::from_ptr(ptr));
> +        }
> +    }
> +
> +    pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
>          Stream { ptr: Some(ptr) }
>      }

Yes I think using 'from_ptr' would make more sense. We probably have
to update that everywhere also I don't think we want have that as
public.

Can you just remove the 'pub' flag of that function?

> @@ -126,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 {

Same comment about usage of 'unsafe'.

> +            virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int)

We defined StreamEventType as unsigned.

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

Same about unsafe.

> +            if virStreamEventRemoveCallback(self.as_ptr()) == -1 {
> +                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.0