On 1/13/20 3:53 AM, Sahid Orentino Ferdjaoui wrote:
> On Fri, Jan 10, 2020 at 11:35:32AM -0700, Zixing Liu wrote:
>> On 2020-01-10 04:48, Sahid Orentino Ferdjaoui wrote:
>>
>>> On Tue, Dec 24, 2019 at 12:12:53AM -0700, Zixing Liu wrote:
>>>> * used rustfmt to clean up the code
>>>>
>>>> Signed-off-by: Zixing Liu <liushuyu@xxxxxxx>
>>>> ---
>>>> src/stream.rs | 97 +++++++++++++++++++++++++++++----------------------
>>>> 1 file changed, 56 insertions(+), 41 deletions(-)
>>> NAK, we don't have specific rule for that. Even if I'm OK with you to
>>> use rustfmt we should consider having CI to also check that.
>> Well, it's also one part of my habit since formatted code is more
>> readable and looks neat. Speaking of CI, I think you could
>> integrate the format check or even commit format changes in CI using
>> `cargo-fmt` which comes with every Rust installation.
> Do you think you can add with this patch a change in CI so that format
> of src/stream.rs will be verified? So basically we will avoid
> regression and we will be able to clean the code incrementally.
Well, I guess I can add the patch to check the format in the changed
files from the last commit in the CI system. However, I am not quite
sure after merging a multi-commit patch, what changes would the
`HEAD^...HEAD` include.
Also, in this patch series, I changed more than one file. The formatter
will check the entire file in which the changes had occurred instead of
the diff since the format check is contextual in Rust.
And are there any other problems with this patch-set that I haven't
addressed?
>>>> diff --git a/src/stream.rs b/src/stream.rs
>>>> index 1ffd186..0d84fd7 100644
>>>> --- a/src/stream.rs
>>>> +++ b/src/stream.rs
>>>> @@ -18,8 +18,8 @@
>>>>
>>>> extern crate libc;
>>>>
>>>> -use connect::Connect;
>>>> use connect::sys::virConnectPtr;
>>>> +use connect::Connect;
>>>> use std::convert::TryFrom;
>>>>
>>>> use error::Error;
>>>> @@ -33,28 +33,28 @@ pub mod sys {
>>>>
>>>> #[link(name = "virt")]
>>>> extern "C" {
>>>> - fn virStreamNew(c: virConnectPtr,
>>>> - flags: libc::c_uint)
>>>> - -> sys::virStreamPtr;
>>>> - fn virStreamSend(c: sys::virStreamPtr,
>>>> - data: *const libc::c_char,
>>>> - nbytes: libc::size_t)
>>>> - -> libc::c_int;
>>>> - fn virStreamRecv(c: sys::virStreamPtr,
>>>> - data: *mut libc::c_char,
>>>> - nbytes: libc::size_t)
>>>> - -> libc::c_int;
>>>> + fn virStreamNew(c: virConnectPtr, flags: libc::c_uint) -> sys::virStreamPtr;
>>>> + fn virStreamSend(
>>>> + c: sys::virStreamPtr,
>>>> + data: *const libc::c_char,
>>>> + nbytes: libc::size_t,
>>>> + ) -> libc::c_int;
>>>> + fn virStreamRecv(
>>>> + c: sys::virStreamPtr,
>>>> + data: *mut libc::c_char,
>>>> + nbytes: libc::size_t,
>>>> + ) -> libc::c_int;
>>>> fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>>>> fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>>>> fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
>>>> - fn virStreamEventAddCallback(c: sys::virStreamPtr,
>>>> - event: libc::c_int,
>>>> - callback: StreamEventCallback,
>>>> - opaque: *const libc::c_void,
>>>> - ff: FreeCallback)
>>>> - -> libc::c_int;
>>>> - fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
>>>> - events: libc::c_int) -> libc::c_int;
>>>> + fn virStreamEventAddCallback(
>>>> + c: sys::virStreamPtr,
>>>> + event: libc::c_int,
>>>> + callback: StreamEventCallback,
>>>> + opaque: *const libc::c_void,
>>>> + ff: FreeCallback,
>>>> + ) -> libc::c_int;
>>>> + fn virStreamEventUpdateCallback(c: sys::virStreamPtr, events: libc::c_int) -> libc::c_int;
>>>> fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
>>>> }
>>>>
>>>> @@ -71,14 +71,16 @@ pub type StreamEventCallback = extern "C" fn(sys::virStreamPtr, libc::c_int, *co
>>>> pub type FreeCallback = extern "C" fn(*mut libc::c_void);
>>>>
>>>> // wrapper for callbacks
>>>> -extern "C" fn event_callback(c: sys::virStreamPtr, flags: libc::c_int, opaque: *const libc::c_void) {
>>>> - let flags = flags as StreamFlags;
>>>> - let shadow_self = unsafe {
>>>> - &mut*(opaque as *mut Stream)
>>>> - };
>>>> - if let Some(callback) = &mut shadow_self.callback {
>>>> - callback(&Stream::from_ptr(c), flags);
>>>> - }
>>>> +extern "C" fn event_callback(
>>>> + c: sys::virStreamPtr,
>>>> + flags: libc::c_int,
>>>> + opaque: *const libc::c_void,
>>>> +) {
>>>> + let flags = flags as StreamFlags;
>>>> + let shadow_self = unsafe { &mut *(opaque as *mut Stream) };
>>>> + if let Some(callback) = &mut shadow_self.callback {
>>>> + callback(&Stream::from_ptr(c), flags);
>>>> + }
>>>> }
>>>>
>>>> extern "C" fn event_free(_opaque: *mut libc::c_void) {}
>>>> @@ -93,16 +95,18 @@ impl Drop for Stream {
>>>> fn drop(&mut self) {
>>>> if self.ptr.is_some() {
>>>> if let Err(e) = self.free() {
>>>> - panic!("Unable to drop memory for Stream, code {}, message: {}",
>>>> - e.code,
>>>> - e.message)
>>>> + panic!(
>>>> + "Unable to drop memory for Stream, code {}, message: {}",
>>>> + e.code, e.message
>>>> + )
>>>> }
>>>> }
>>>> if self.callback.is_some() {
>>>> if let Err(e) = self.event_remove_callback() {
>>>> - panic!("Unable to remove event callback for Stream, code {}, message: {}",
>>>> - e.code,
>>>> - e.message)
>>>> + panic!(
>>>> + "Unable to remove event callback for Stream, code {}, message: {}",
>>>> + e.code, e.message
>>>> + )
>>>> }
>>>> }
>>>> }
>>>> @@ -120,7 +124,10 @@ impl Stream {
>>>> }
>>>>
>>>> pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
>>>> - Stream { ptr: Some(ptr), callback: None }
>>>> + Stream {
>>>> + ptr: Some(ptr),
>>>> + callback: None,
>>>> + }
>>>> }
>>>>
>>>> pub fn as_ptr(&self) -> sys::virStreamPtr {
>>>> @@ -160,7 +167,7 @@ impl Stream {
>>>> virStreamSend(
>>>> self.as_ptr(),
>>>> data.as_ptr() as *mut libc::c_char,
>>>> - data.len()
>>>> + data.len(),
>>>> )
>>>> };
>>>> usize::try_from(ret).map_err(|_| Error::new())
>>>> @@ -177,10 +184,20 @@ impl Stream {
>>>> usize::try_from(ret).map_err(|_| Error::new())
>>>> }
>>>>
>>>> - pub fn event_add_callback<F: 'static + FnMut(&Stream, StreamEventType)>(&mut self, events: StreamEventType, cb: F) -> Result<(), Error> {
>>>> + pub fn event_add_callback<F: 'static + FnMut(&Stream, StreamEventType)>(
>>>> + &mut self,
>>>> + events: StreamEventType,
>>>> + cb: F,
>>>> + ) -> Result<(), Error> {
>>>> let ret = unsafe {
>>>> let ptr = &*self as *const _ as *const _;
>>>> - virStreamEventAddCallback(self.as_ptr(), events as libc::c_int, event_callback, ptr, event_free)
>>>> + virStreamEventAddCallback(
>>>> + self.as_ptr(),
>>>> + events as libc::c_int,
>>>> + event_callback,
>>>> + ptr,
>>>> + event_free,
>>>> + )
>>>> };
>>>> if ret == -1 {
>>>> return Err(Error::new());
>>>> @@ -190,9 +207,7 @@ impl Stream {
>>>> }
>>>>
>>>> pub fn event_update_callback(&self, events: StreamEventType) -> Result<(), Error> {
>>>> - let ret = unsafe {
>>>> - virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int)
>>>> - };
>>>> + let ret = unsafe { virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int) };
>>>> if ret == -1 {
>>>> return Err(Error::new());
>>>> }
>>>> --
>>>> 2.24.1
