* pass same size to virStreamRecv as the buffer has allocated * Handle -2 error case * Fix FFI declaration to take size_t instead of c_uint * Allow user to pass in buffer. To allow user to decide where to allocate it. And to be able to re-use the same buffer * Don't try to treat binary data as a string Signed-off-by: Linus Färnstrand <faern(a)faern.net&gt; --- src/stream.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index 8333ee5..af6c8ec 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -18,6 +18,7 @@ extern crate libc; +use std::convert::TryFrom; use std::str; use error::Error; @@ -37,7 +38,7 @@ extern "C" { -> libc::c_int; fn virStreamRecv(c: sys::virStreamPtr, data: *mut libc::c_char, - nbytes: libc::c_uint) + nbytes: libc::size_t) -> libc::c_int; fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int; fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int; @@ -116,14 +117,14 @@ impl Stream { } } - pub fn recv(&self, size: u32) -> Result<String, Error> { - unsafe { - let mut data: [libc::c_char; 2048] = ['\0' as i8; 2048]; - let ret = virStreamRecv(self.as_ptr(), data.as_mut_ptr(), size as libc::c_uint); - if ret == -1 { - return Err(Error::new()); - } - return Ok(c_chars_to_string!(data.as_ptr())); - } + pub fn recv(&self, buf: &mut [u8]) -> Result<usize, Error> { + let ret = unsafe { + virStreamRecv( + self.as_ptr(), + buf.as_mut_ptr() as *mut libc::c_char, + buf.len(), + ) + }; + usize::try_from(ret).map_err(|_| Error::new())

} } -- 2.21.0 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, September 20, 2019 9:58 AM, Martin Kletzander <mkletzan(a)redhat.com&gt; wrote: > On Thu, Sep 19, 2019 at 05:48:37AM +0000, Linus Färnstrand wrote: > > > - pass same size to virStreamRecv as the buffer has allocated > > - Handle -2 error case > > - Fix FFI declaration to take size_t instead of c_uint > > - Allow user to pass in buffer. To allow user to decide where to > > allocate it. And to be able to re-use the same buffer > > > > - Don't try to treat binary data as a string > > > > Signed-off-by: Linus Färnstrand faern(a)faern.net > > > > ------------------------------------------------ > > > > src/stream.rs | 21 +++++++++++---------- > > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/src/stream.rs b/src/stream.rs > > index 8333ee5..af6c8ec 100644 > > --- a/src/stream.rs > > +++ b/src/stream.rs > > @@ -18,6 +18,7 @@ > > extern crate libc; > > +use std::convert::TryFrom; > > use std::str; > > use error::Error; > > @@ -37,7 +38,7 @@ extern "C" { > > -> libc::c_int; > > fn virStreamRecv(c: sys::virStreamPtr, > > data: *mut libc::c_char, > > > > - nbytes: libc::c_uint) > > > > > > > > - nbytes: libc::size_t) > > -> libc::c_int; > > > > > > fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int; > > fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int; > > @@ -116,14 +117,14 @@ impl Stream { > > } > > } > > > > > > - pub fn recv(&self, size: u32) -> Result<String, Error> { > > - unsafe { > > > > > > - let mut data: [libc::c_char; 2048] = ['\\0' as i8; 2048]; > > > > > > - let ret = virStreamRecv(self.as_ptr(), data.as_mut_ptr(), size as libc::c_uint); > > > > > > - if ret == -1 { > > > > > > - return Err(Error::new()); > > > > > > - } > > > > > > - return Ok(c_chars_to_string!(data.as_ptr())); > > > > > > - } > > > > > > > > - pub fn recv(&self, buf: &mut [u8]) -> Result<usize, Error> { > > - let ret = unsafe { > > > > > > - virStreamRecv( > > > > > > - self.as_ptr(), > > > > > > - buf.as_mut_ptr() as *mut libc::c_char, > > > > > > - buf.len(), > > > > > > - ) > > > > > > - }; > > > > > > - usize::try_from(ret).map_err(|_| Error::new()) > > > > > > This way it is impossible to distinguish -1 and -2. We could get a bit closer > to std::io::Read if -2 gets transformed to 0, as we haven't read anything. I thought the libvirt error would be set to something appropriate in the -2 case. Maybe I'm wrong. The problem is that the error type currently is very opaque. I wanted to also make the error type better, but that felt out of scope for this patch. I don't think just transforming -2 into 0 is very good, since that also does not allow distinguish that error from nothing read.

IMO, the best would be to make Error into a more expressive type with something similar to std::io::ErrorKind.

> > Other than that it's good. > > > } > > } > > > > ---- > > > > 2.21.0 > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list