On Wed, Jul 17, 2019 at 08:59:30AM +0000, Sahid Orentino Ferdjaoui wrote:
In this refactor we avoid to enclose all the code with unsafe tags and
just use it when necessary. Also we add annotations to explain why
it's safe to use.
The integrations tests related are also been reviewed to avoid using
panic.
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@xxxxxxxxxxxxx>
---
src/connect.rs | 91 ++++++++++++++++++++++-----------------
tests/integration_qemu.rs | 16 +++----
2 files changed, 56 insertions(+), 51 deletions(-)
diff --git a/src/connect.rs b/src/connect.rs
index 0fa8551..108224d 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -240,6 +240,41 @@ extern "C" {
-> *mut libc::c_char;
}
+extern "C" fn connectCallback(ccreds: sys::virConnectCredentialPtr,
+ ncred: libc::c_uint,
+ cbdata: *mut libc::c_void)
+ -> libc::c_int {
+ let callback: ConnectAuthCallback = unsafe {
+ // Safe because connectCallback is private and only used by
+ // Connect::open_auth(). In open_auth() we transmute the
+ // callback allocate in *void.
+ mem::transmute(cbdata)
+ };
+ let mut rcreds: Vec<ConnectCredential> = Vec::new();
+ for i in 0..ncred as isize {
+ unsafe {
+ // Safe because ccreds is allocated.
+ let c = ConnectCredential::from_ptr(ccreds.offset(i));
+ rcreds.push(c);
+ }
+ }
+ callback(&mut rcreds);
+ for i in 0..ncred as isize {
+ if rcreds[i as usize].result.is_some() {
+ if let Some(ref result) = rcreds[i as usize].result {
+ unsafe {
+ // Safe because ccreds is allocated and the result
+ // is comming from Rust calls.
+ (*ccreds.offset(i)).resultlen = result.len() as libc::c_uint;
+ (*ccreds.offset(i)).result = string_to_mut_c_chars!(result.clone());
+ }
+ }
+ }
+ }
+ 0
+}
+
+
pub type ConnectFlags = self::libc::c_uint;
pub const VIR_CONNECT_RO: ConnectFlags = 1 << 0;
pub const VIR_CONNECT_NO_ALIASES: ConnectFlags = 1 << 1;
@@ -412,39 +447,6 @@ impl ConnectAuth {
callback: callback,
}
}
-
- fn to_cstruct(&mut self) -> sys::virConnectAuth {
- unsafe extern "C" fn wrapper(ccreds: sys::virConnectCredentialPtr,
- ncred: libc::c_uint,
- cbdata: *mut libc::c_void)
- -> libc::c_int {
- let callback: ConnectAuthCallback = mem::transmute(cbdata);
- let mut rcreds: Vec<ConnectCredential> = Vec::new();
- for i in 0..ncred as isize {
- let c = ConnectCredential::from_ptr(ccreds.offset(i));
- rcreds.push(c);
- }
- callback(&mut rcreds);
- for i in 0..ncred as isize {
- if rcreds[i as usize].result.is_some() {
- if let Some(ref result) = rcreds[i as usize].result {
- (*ccreds.offset(i)).resultlen = result.len() as libc::c_uint;
- (*ccreds.offset(i)).result = string_to_mut_c_chars!(result.clone());
- }
- }
- }
- 0
- }
-
- unsafe {
- sys::virConnectAuth {
- credtype: &mut self.creds[0],
- ncredtype: self.creds.len() as u32,
- cb: wrapper,
- cbdata: mem::transmute(self.callback),
- }
- }
- }
}
/// Provides APIs for the management of hosts.
@@ -554,14 +556,23 @@ impl Connect {
auth: &mut ConnectAuth,
flags: ConnectFlags)
-> Result<Connect, Error> {
- unsafe {
- let mut cauth = auth.to_cstruct();
- let c = virConnectOpenAuth(string_to_c_chars!(uri), &mut cauth, flags as libc::c_uint);
- if c.is_null() {
- return Err(Error::new());
- }
- return Ok(Connect::new(c));
+ let mut cauth = unsafe {
+ // Safe because Rust forces to allocate all attributes of
+ // the struct ConnectAuth.
+ sys::virConnectAuth {
+ credtype: &mut auth.creds[0],
+ ncredtype: auth.creds.len() as libc::c_uint,
+ cb: connectCallback,
+ cbdata: mem::transmute(auth.callback),
+ }
+ };
+ let c = unsafe {
+ virConnectOpenAuth(string_to_c_chars!(uri), &mut cauth, flags as libc::c_uint)
+ };
+ if c.is_null() {
+ return Err(Error::new());
}
+ return Ok(Connect::new(c));
}
diff --git a/tests/integration_qemu.rs b/tests/integration_qemu.rs
index 49e07c4..79aa2bd 100644
--- a/tests/integration_qemu.rs
+++ b/tests/integration_qemu.rs
@@ -108,14 +108,9 @@ fn test_connection_with_auth() {
let mut auth = ConnectAuth::new(vec![::virt::connect::VIR_CRED_AUTHNAME,
::virt::connect::VIR_CRED_PASSPHRASE],
callback);
- match Connect::open_auth("test+tcp://127.0.0.1/default", &mut auth, 0) {
- Ok(c) => common::close(c),
- Err(e) => {
- panic!("open_auth did not work: code {}, message: {}",
- e.code,
- e.message)
- }
- }
+ let c = Connect::open_auth("test+tcp://127.0.0.1/default", &mut auth, 0);
+ assert_eq!(true, c.is_ok());
Wouldn't it make more sense to do: assert!(c.is_ok());
+ common::close(c.unwrap());
}
@@ -141,9 +136,8 @@ fn test_connection_with_auth_wrong() {
let mut auth = ConnectAuth::new(vec![::virt::connect::VIR_CRED_AUTHNAME,
::virt::connect::VIR_CRED_PASSPHRASE],
callback);
- if Connect::open_auth("test+tcp://127.0.0.1/default", &mut auth, 0).is_ok() {
- panic!("open_auth did not work: code {}, message:");
- }
+ let c = Connect::open_auth("test+tcp://127.0.0.1/default", &mut auth, 0);
+ assert_eq!(false, c.is_ok());
Same here with: assert!(c.is_err()); ???
} #[test] -- 2.17.1 -- 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
