Fix some make check/memcheck issues

build-aux/knet_valgrind_memcheck.supp

@@ -727,3 +727,77 @@
   fun:start_thread
   fun:clone
 }
+{
+   For knet-test Rust API checker
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   fun:_ZN5alloc5alloc5alloc17h88e9509a359ebb46E
+   fun:_ZN5alloc5alloc6Global10alloc_impl17heec423e473c84c77E
+   fun:_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$8allocate17hf695ca6d80143d73E
+   fun:_ZN9hashbrown3raw5alloc5inner8do_alloc17ha7cf839e4ae3b605E
+   fun:_ZN9hashbrown3raw22RawTableInner$LT$A$GT$17new_uninitialized17h2f8ed9ed97d99c59E
+   fun:_ZN9hashbrown3raw22RawTableInner$LT$A$GT$22fallible_with_capacity17h463cef2f6ab21358E
+   fun:_ZN9hashbrown3raw22RawTableInner$LT$A$GT$14prepare_resize17h167fb9f59c4c3769E
+   fun:_ZN9hashbrown3raw21RawTable$LT$T$C$A$GT$6resize17h220394e4ddf95cdaE
+   fun:_ZN9hashbrown3raw21RawTable$LT$T$C$A$GT$14reserve_rehash17hdb1acfe6616e110bE
+   fun:_ZN9hashbrown3raw21RawTable$LT$T$C$A$GT$7reserve17h6adf3b6cbced4ff9E
+   fun:_ZN9hashbrown3raw21RawTable$LT$T$C$A$GT$6insert17h04df28ed06102244E
+}
+{
+   For knet-test Rust API checker
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   fun:UnknownInlinedFun
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:pthread_create
+   fun:_ZN3std3sys4unix6thread6Thread3new17hd28665ab47e6837bE
+   fun:_ZN3std6thread7Builder15spawn_unchecked17h68cc146c8b166f39E
+   fun:_ZN3std6thread7Builder5spawn17hcd1b38d2384c060eE
+   fun:_ZN3std6thread5spawn17h3b50460b84f53569E
+   fun:_ZN9knet_test10setup_node17h60e4258a4f1b5542E
+   fun:_ZN9knet_test4main17h93dc1eb52d872352E
+   fun:_ZN4core3ops8function6FnOnce9call_once17h992aa9d4709882fdE
+   fun:_ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17hbe9958990c11224dE
+   fun:_ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17h056af6a930923e23E
+}
+{
+   For knet-test Rust API checker
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   fun:UnknownInlinedFun
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:pthread_create
+   fun:_ZN3std3sys4unix6thread6Thread3new17hd28665ab47e6837bE
+   fun:_ZN3std6thread7Builder15spawn_unchecked17h68cc146c8b166f39E
+   fun:_ZN3std6thread7Builder5spawn17hcd1b38d2384c060eE
+   fun:_ZN3std6thread5spawn17h3b50460b84f53569E
+   fun:_ZN9knet_test10setup_node17h60e4258a4f1b5542E
+   fun:_ZN9knet_test4main17h93dc1eb52d872352E
+   fun:_ZN4core3ops8function6FnOnce9call_once17h992aa9d4709882fdE
+   fun:_ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17hbe9958990c11224dE
+   fun:_ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17h056af6a930923e23E
+}
+{
+   For knet-test Rust API checker
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   fun:UnknownInlinedFun
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:pthread_create
+   fun:_ZN3std3sys4unix6thread6Thread3new17hd28665ab47e6837bE
+   fun:_ZN3std6thread7Builder15spawn_unchecked17ha6d7eea36215382dE
+   fun:_ZN3std6thread7Builder5spawn17h26a34bb5ab6cd4f8E
+   fun:_ZN3std6thread5spawn17hefbe7bfaa873c479E
+   fun:_ZN9kronosnet7libknet10handle_new17h7ab5f5a37566b38eE
+   fun:_ZN9knet_test10setup_node17h60e4258a4f1b5542E
+   fun:_ZN9knet_test4main17h93dc1eb52d872352E
+   fun:_ZN4core3ops8function6FnOnce9call_once17h992aa9d4709882fdE
+   fun:_ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17hbe9958990c11224dE
+}

libknet/bindings/rust/build.rs.in

@@ -5,8 +5,6 @@
 // Author: Christine Caulfield (ccaulfi@redhat.com)
 //
 
-extern crate pkg_config;
-
 fn main() {
     println!("cargo:rustc-link-search=native=../../");
     println!("cargo:rustc-link-lib=knet");

libknet/bindings/rust/src/lib.rs

@@ -102,27 +102,32 @@ fn string_from_bytes(bytes: *const ::std::os::raw::c_char, max_length: usize) ->
     let mut newbytes = Vec::<u8>::new();
     newbytes.resize(max_length, 0u8);
 
-    unsafe {
-	// We need to fully copy it, not shallow copy it.
-	// Messy casting on both parts of the copy here to get it to work on both signed
-	// and unsigned char machines
-	copy_nonoverlapping(bytes as *mut i8, newbytes.as_mut_ptr() as *mut i8, max_length);
-    }
-
     // Get length of the string in old-fashioned style
     let mut length: usize = 0;
-    for (count, i) in newbytes.iter().enumerate() {
-	if *i == 0 && length == 0 {
+    let mut count = 0;
+    let mut tmpbytes = bytes;
+    while count < max_length || length == 0 {
+	if unsafe {*tmpbytes} == 0 && length == 0 {
 	    length = count;
 	    break;
 	}
+	count += 1;
+	tmpbytes = unsafe { tmpbytes.offset(1) }
     }
 
     // Cope with an empty string
     if length == 0 {
 	return Ok(String::new());
     }
 
+    unsafe {
+	// We need to fully copy it, not shallow copy it.
+	// Messy casting on both parts of the copy here to get it to work on both signed
+	// and unsigned char machines
+	copy_nonoverlapping(bytes as *mut i8, newbytes.as_mut_ptr() as *mut i8, length);
+    }
+
+
     let cs = CString::new(&newbytes[0..length as usize])?;
 
     // This is just to convert the error type
@@ -141,7 +146,7 @@ fn string_from_bytes_safe(bytes: *const ::std::os::raw::c_char, max_length: usiz
     }
 }
 
-fn string_to_bytes(s: &str, bytes: &mut [c_char]) ->Result<()>
+fn string_to_bytes(s: &str, bytes: &mut [c_char]) -> Result<()>
 {
     let c_name = match CString::new(s) {
 	Ok(n) => n,
@@ -151,9 +156,10 @@ fn string_to_bytes(s: &str, bytes: &mut [c_char]) ->Result<()>
     if c_name.as_bytes().len() > bytes.len() {
 	return Err(Error::new(ErrorKind::Other, "String too long"));
     }
+
     unsafe {
 	// NOTE param order is 'wrong-way round' from C
-	copy_nonoverlapping(c_name.as_ptr(), bytes.as_mut_ptr(), bytes.len());
+	copy_nonoverlapping(c_name.as_ptr(), bytes.as_mut_ptr(), c_name.as_bytes().len());
     }
     Ok(())
 }

libknet/bindings/rust/src/libknet.rs

@@ -13,7 +13,7 @@ use crate::sys::libknet as ffi;
 
 use std::ffi::{CString, CStr};
 use std::sync::mpsc::*;
-use std::ptr::{copy_nonoverlapping, null};
+use std::ptr::{copy_nonoverlapping, null, null_mut};
 use std::sync::Mutex;
 use std::collections::HashMap;
 use std::io::{Result, Error, ErrorKind};
@@ -1554,23 +1554,29 @@ pub fn link_set_config(handle: Handle, host_id: &HostId, link_id: u8,
 		       src_addr: &SocketAddr, dst_addr: Option<&SocketAddr>, flags: LinkFlags) -> Result<()>
 {
     // Not really mut, but C is dumb
-    let mut c_srcaddr : OsSocketAddr = (*src_addr).into();
+    let mut c_srcaddr = make_new_sockaddr_storage(src_addr);
 
     // dst_addr can be NULL/None if this is a dynamic link
-    let c_dstaddr = if let Some(dst) = dst_addr {
-	let c_dst : OsSocketAddr = (*dst).into();
-	c_dst.as_ptr()
-    } else {
-	null()
-    };
+    let res = if let Some(dst) = dst_addr {
+	let mut c_dstaddr = make_new_sockaddr_storage(dst);
 
-    let res = unsafe {
-	ffi::knet_link_set_config(handle.knet_handle as ffi::knet_handle_t,
-				  host_id.host_id, link_id,
-				  transport.to_u8(),
-				  c_srcaddr.as_mut_ptr() as *mut ffi::sockaddr_storage,
-				  c_dstaddr as *mut ffi::sockaddr_storage,
-				  flags.bits)
+	unsafe {
+	    ffi::knet_link_set_config(handle.knet_handle as ffi::knet_handle_t,
+				      host_id.host_id, link_id,
+				      transport.to_u8(),
+				      &mut c_srcaddr,
+				      &mut c_dstaddr,
+				      flags.bits)
+	}
+    } else {
+	unsafe {
+	    ffi::knet_link_set_config(handle.knet_handle as ffi::knet_handle_t,
+				      host_id.host_id, link_id,
+				      transport.to_u8(),
+				      &mut c_srcaddr,
+				      null_mut(),
+				      flags.bits)
+	}
     };
     if res == 0 {
 	Ok(())

libknet/bindings/rust/tests/src/bin/knet-test.rs

@@ -304,14 +304,12 @@ fn close_handle(handle: knet::Handle, remnode: u16) -> Result<()>
 	return Err(e);
     }
 
-
-
     if let Err(e) = knet::link_set_enable(handle, &other_hostid, 0, false) {
 	println!("Error from set_link_enable(false): {}", e);
 	return Err(e);
     }
 
-    if let Err(e) =knet::link_clear_config(handle, &other_hostid, 0) {
+    if let Err(e) = knet::link_clear_config(handle, &other_hostid, 0) {
 	println!("clear config failed: {}", e);
 	return Err(e);
     }
@@ -682,7 +680,6 @@ fn test_acl(handle: knet::Handle, host: &knet::HostId) -> Result<()>
 	return Err(e);
     }
 
-
     // These ACLs are nonsense on stilts
     if let Err(e) = knet::link_add_acl(handle, host, 1,
 				       &SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8003_u16),
@@ -712,7 +709,7 @@ fn test_acl(handle: knet::Handle, host: &knet::HostId) -> Result<()>
     }
 
     // Get rid of this link before it messes things up
-    if let Err(e) =knet::link_clear_config(handle, host, 1) {
+    if let Err(e) = knet::link_clear_config(handle, host, 1) {
 	println!("clear config (dynamic) failed: {}", e);
 	return Err(e);
     }

libknet/tests/api-test-coverage

@@ -80,6 +80,7 @@ done
 # Check Rust bindings coverage
 rust_found=0
 rust_missing=0
+# These are implemented directly in the Rust code
 deliberately_missing="knet_strtoaddr knet_addrtostr knet_get_transport_name_by_id knet_get_transport_id_by_name"
 rustapicalls=$numapicalls
 for i in $headerapicalls; do

libnozzle/bindings/rust/src/lib.rs

@@ -61,27 +61,32 @@ fn string_from_bytes(bytes: *const ::std::os::raw::c_char, max_length: usize) ->
     let mut newbytes = Vec::<u8>::new();
     newbytes.resize(max_length, 0u8);
 
-    unsafe {
-	// We need to fully copy it, not shallow copy it.
-	// Messy casting on both parts of the copy here to get it to work on both signed
-	// and unsigned char machines
-	copy_nonoverlapping(bytes as *mut i8, newbytes.as_mut_ptr() as *mut i8, max_length);
-    }
-
     // Get length of the string in old-fashioned style
     let mut length: usize = 0;
-    for (count, i) in newbytes.iter().enumerate() {
-	if *i == 0 && length == 0 {
+    let mut count = 0;
+    let mut tmpbytes = bytes;
+    while count < max_length || length == 0 {
+	if unsafe {*tmpbytes} == 0 && length == 0 {
 	    length = count;
 	    break;
 	}
+	count += 1;
+	tmpbytes = unsafe { tmpbytes.offset(1) }
     }
 
     // Cope with an empty string
     if length == 0 {
 	return Ok(String::new());
     }
 
+    unsafe {
+	// We need to fully copy it, not shallow copy it.
+	// Messy casting on both parts of the copy here to get it to work on both signed
+	// and unsigned char machines
+	copy_nonoverlapping(bytes as *mut i8, newbytes.as_mut_ptr() as *mut i8, length);
+    }
+
+
     let cs = CString::new(&newbytes[0..length as usize])?;
 
     // This is just to convert the error type
@@ -100,7 +105,7 @@ fn string_from_bytes_safe(bytes: *const ::std::os::raw::c_char, max_length: usiz
     }
 }
 
-fn string_to_bytes(s: &str, bytes: &mut [c_char]) ->Result<()>
+fn string_to_bytes(s: &str, bytes: &mut [c_char]) -> Result<()>
 {
     let c_name = match CString::new(s) {
 	Ok(n) => n,
@@ -110,9 +115,10 @@ fn string_to_bytes(s: &str, bytes: &mut [c_char]) ->Result<()>
     if c_name.as_bytes().len() > bytes.len() {
 	return Err(Error::new(ErrorKind::Other, "String too long"));
     }
+
     unsafe {
 	// NOTE param order is 'wrong-way round' from C
-	copy_nonoverlapping(c_name.as_ptr(), bytes.as_mut_ptr(), bytes.len());
+	copy_nonoverlapping(c_name.as_ptr(), bytes.as_mut_ptr(), c_name.as_bytes().len());
     }
     Ok(())
 }

libnozzle/bindings/rust/src/libnozzle.rs

@@ -289,7 +289,8 @@ pub fn get_mac(handle: Handle) -> Result<String>
 	ffi::nozzle_get_mac(handle.nozzle_handle, &mut c_mac)
     };
     if res == 0 {
-	let mac = crate::string_from_bytes(c_mac, 19_usize)?;
+	let mac = crate::string_from_bytes(c_mac, 24_usize)?; // Needs to be 8byte aligned
+	unsafe { free(c_mac as *mut c_void); }// Was created with strdup(
 	Ok(mac)
     } else {
 	Err(Error::last_os_error())
@@ -299,7 +300,7 @@ pub fn get_mac(handle: Handle) -> Result<String>
 /// Setsthe MAC address of the device
 pub fn set_mac(handle: Handle, ether_addr: &str) -> Result<()>
 {
-    let mut c_mac: [c_char; 19_usize] = [0; 19_usize];
+    let mut c_mac: [c_char; 24_usize] = [0; 24_usize]; // Needs to be 8byte aligned
     crate::string_to_bytes(&ether_addr, &mut c_mac)?;
     let res = unsafe {
 	ffi::nozzle_set_mac(handle.nozzle_handle, c_mac.as_ptr())

libnozzle/bindings/rust/tests/src/bin/nozzle-test.rs

@@ -10,12 +10,12 @@
 use nozzle::libnozzle as nozzle;
 use std::io::{Result, Error, ErrorKind, BufWriter, Write};
 use std::fmt::Write as fmtwrite;
-use std::env;
 use std::{thread, time};
 use std::fs::File;
 use std::fs;
 
 const SKIP: i32 = 77;
+const RUN_DIR : &str = "/tmp";
 
 fn main() -> Result<()>
 {
@@ -24,8 +24,11 @@ fn main() -> Result<()>
 	std::process::exit(SKIP);
     }
 
+    // Do this in /tmp so we don't soil the filesystem
+    std::env::set_current_dir(RUN_DIR)?;
+
     let mut nozzle_name = String::from("");
-    let handle = match nozzle::open(&mut nozzle_name,  &String::from(env::current_dir().unwrap().to_str().unwrap())) {
+    let handle = match nozzle::open(&mut nozzle_name, RUN_DIR) {
 	Ok(h) => {
 	    println!("Opened device {}", nozzle_name);
 	    h
@@ -81,7 +84,7 @@ fn main() -> Result<()>
 	return Err(e);
     }
 
-    // Create the 'up' script so we can test the run_updown() function
+    // Create the 'up' script so we can test the run_updown() function,
     let up_path = std::path::Path::new("up.d");
     if let Err(e) = fs::create_dir_all(up_path) {
 	eprintln!("Error creating up.d directory: {:?}", e);
@@ -105,7 +108,7 @@ fn main() -> Result<()>
     }
     // A grotty way to do chmod, but normally this would be distributed by the sysadmin
     unsafe {
-	let up_cstring = std::ffi::CString::new(up_filename).unwrap();
+	let up_cstring = std::ffi::CString::new(up_filename.clone()).unwrap();
 	libc::chmod(up_cstring.as_ptr(), 0o700);
     }
 
@@ -117,6 +120,9 @@ fn main() -> Result<()>
 	}
     }
 
+    // Tidy up after ourself - remove the up.d/tapX file
+    fs::remove_file(&up_filename)?;
+    fs::remove_dir(&"up.d")?;
 
     match nozzle::get_ips(handle) {
 	Ok(ips) => {