‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, September 20, 2019 9:58 AM, Martin Kletzander <mkletzan@xxxxxxxxxx> 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@xxxxxxxxx
> >
> > ------------------------------------------------
> >
> > 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@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
