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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 16, 2019 at 01:49:51PM +0200, Sahid Orentino Ferdjaoui wrote:
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@xxxxxxxxx>
> ---
> 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.


Sure, go ahead.

Thanks,
s.

>     }
>
>     pub fn recv(&self, buf: &mut [u8]) -> Result<usize, Error> {
> --
> 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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux