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

[Zixing Liu]

On 2020-01-10 04:36, Sahid Orentino Ferdjaoui wrote:

> 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().

Yes, and the change was included in this series of patches, patch number 05.

For the comment, should I argue about the pointer in question is mostly safe since it's coming from the underlying `libvirt` library?

>> +            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