Re: [libvirt] [PATCH 1/2] libvirt-rust: Fix bugs in Stream::recv

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.

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