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@xxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxx>
>
>> 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@xxxxxxxxxx>
>> + */
>> +
>> +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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
