Re: [libvirt] [PATCH 2/2] libvirt-rust Fix Stream::send

On Fri, Sep 20, 2019 at 10:02:47AM +0200, Martin Kletzander wrote: > On Thu, Sep 19, 2019 at 05:48:49AM +0000, Linus Färnstrand wrote: > > * Handle the -2 error case > > * Allow sending arbitrary byte array, not just UTF-8 strings > > * Fix FFI declaration, takes size_t, not c_uint > > * Return usize to be more idiomatic (type used for slice indexing) > > > > Signed-off-by: Linus Färnstrand <faern(a)faern.net&gt; > > --- > > src/stream.rs | 21 ++++++++++----------- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/src/stream.rs b/src/stream.rs > > index af6c8ec..2ca5d0b 100644 > > --- a/src/stream.rs > > +++ b/src/stream.rs > > @@ -34,7 +34,7 @@ pub mod sys { > > extern "C" { > > fn virStreamSend(c: sys::virStreamPtr, > > data: *const libc::c_char, > > - nbytes: libc::c_uint) > > + nbytes: libc::size_t) > > -> libc::c_int; > > fn virStreamRecv(c: sys::virStreamPtr, > > data: *mut libc::c_char, > > @@ -105,16 +105,15 @@ impl Stream { > > } > > } > > > > - pub fn send(&self, data: &str) -> Result<u32, Error> { > > - unsafe { > > - let ret = virStreamSend(self.as_ptr(), > > - string_to_c_chars!(data), > > - data.len() as libc::c_uint); > > - if ret == -1 { > > - return Err(Error::new()); > > - } > > - return Ok(ret as u32); > > - } > > + pub fn send(&self, data: &[u8]) -> Result<usize, Error> { > > + let ret = unsafe { > > + virStreamSend( > > + self.as_ptr(), > > + data.as_ptr() as *mut libc::c_char, > > + data.len() > > + ) > > + }; > > + usize::try_from(ret).map_err(|_| Error::new()) > > I guess same as the comment form the first patch should be applied here, but we > might need to even change the return value so that it is properly distinguished > as here it has yet another meaning. Are you basing this on some of your work on > top of libvirt-rust? Is this the easy to use (semi-)properly? If yes, we might > just as well keep it this way as it is still way better than what we have now. I think the patch is good as it is now. Of-course distinguish between -1 and -2 would be the ideal but even previously that was not the case. We could still improve it in the future. Thank you Linus for your patch. I will push it Martin if you don't have objection.

Thanks, s. > > } > > > > pub fn recv(&self, buf: &mut [u8]) -> Result<usize, Error> { > > -- > > 2.21.0 > > > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list