Avoid if possible unsafe code in vrp-cli

Author: reinterpretcat

CHANGELOG.md

@@ -6,12 +6,15 @@ All notable changes to this project will be documented in this file.
 ## [Unreleased]
 
 This release brings hierarchical approach for local objective estimation. Additionally, it focuses on refactoring
-and forbidding usafe code.
+to avoid usafe code.
 
 ### Changed
 
 * use hierarchical approach for local objective estimation
-* avoid unsafe code in rosomaxa/vrp-core/vrp-pragmatic/vrp-scientific crates and apply forbid_unsafe_code crate-wise
+* avoid unsafe code project-wise:
+  * apply `#![forbid(unsafe_code)]` directive for rosomaxa/vrp-core/vrp-pragmatic/vrp-scientific
+  * apply `#![deny(unsafe_code)]` directive for vrp-cli (due to FFI which is only one exception)
+  * allow unsafe code only when necessary in research crates which are not used anyhow in production code
 
 ### Added
 

vrp-cli/src/commands/generate.rs

@@ -70,10 +70,10 @@ pub fn run_generate(matches: &ArgMatches) -> Result<(), String> {
     match generate_problem_from_args(matches) {
         Ok((problem, input_format)) => {
             let out_result = matches.get_one::<String>(OUT_RESULT_ARG_NAME).map(|path| create_file(path, "out result"));
-            let out_buffer = create_write_buffer(out_result);
+            let mut out_buffer = create_write_buffer(out_result);
 
             match input_format.as_str() {
-                "pragmatic" => serialize_problem(out_buffer, &problem)
+                "pragmatic" => serialize_problem(&problem, &mut out_buffer)
                     .map_err(|err| format!("cannot serialize as pragmatic problem: '{err}'")),
                 _ => Err(format!("unknown output format: '{input_format}'")),
             }

vrp-cli/src/commands/import.rs

@@ -41,8 +41,9 @@ pub fn run_import(matches: &ArgMatches) -> Result<(), String> {
     match import_problem(input_format, input_files) {
         Ok(problem) => {
             let out_result = matches.get_one::<String>(OUT_RESULT_ARG_NAME).map(|path| create_file(path, "out result"));
-            let out_buffer = create_write_buffer(out_result);
-            serialize_problem(out_buffer, &problem).map_err(|err| format!("cannot serialize result problem: '{err}'"))
+            let mut out_buffer = create_write_buffer(out_result);
+            serialize_problem(&problem, &mut out_buffer)
+                .map_err(|err| format!("cannot serialize result problem: '{err}'"))
         }
         Err(err) => Err(format!("cannot import problem: '{err}'")),
     }

vrp-cli/src/extensions/analyze/clusters.rs

@@ -47,13 +47,15 @@ pub fn get_clusters<F: Read>(
         })
         .collect::<Vec<_>>();
 
-    let mut buffer = String::new();
-    let writer = unsafe { BufWriter::new(buffer.as_mut_vec()) };
+    let mut writer = BufWriter::new(Vec::new());
 
-    serialize_named_locations_as_geojson(writer, locations.as_slice())
+    serialize_named_locations_as_geojson(locations.as_slice(), &mut writer)
         .map_err(|err| format!("cannot write named locations as geojson: '{err}'"))?;
 
-    Ok(buffer)
+    let bytes = writer.into_inner().map_err(|err| format!("{err}"))?;
+    let result = String::from_utf8(bytes).map_err(|err| format!("{err}"))?;
+
+    Ok(result)
 }
 
 fn get_core_problem<F: Read>(

vrp-cli/src/lib.rs

@@ -10,6 +10,7 @@
 //! - `vrp-core` crate implements default metaheuristic
 
 #![warn(missing_docs)]
+#![deny(unsafe_code)] // NOTE: use deny instead forbid as we need allow unsafe code for c_interop
 
 #[cfg(test)]
 #[path = "../tests/helpers/mod.rs"]
@@ -45,6 +46,7 @@ use vrp_pragmatic::get_unique_locations;
 use vrp_pragmatic::validation::ValidationContext;
 
 #[cfg(not(target_arch = "wasm32"))]
+#[allow(unsafe_code)]
 mod c_interop {
     use super::*;
     use crate::extensions::solve::config::read_config;
@@ -110,22 +112,22 @@ mod c_interop {
     extern "C" fn convert_to_pragmatic(
         format: *const c_char,
         inputs: *const *const c_char,
-        input_len: *const i32,
+        input_len: usize,
         success: Callback,
         failure: Callback,
     ) {
         catch_panic(failure, || {
             let format = to_string(format);
-            let inputs = unsafe { slice::from_raw_parts(inputs, input_len as usize).to_vec() };
+            let inputs = unsafe { slice::from_raw_parts(inputs, input_len).to_vec() };
             let inputs = inputs.iter().map(|p| to_string(*p)).collect::<Vec<_>>();
             let readers = inputs.iter().map(|p| BufReader::new(p.as_bytes())).collect::<Vec<_>>();
 
             match import_problem(format.as_str(), Some(readers)) {
                 Ok(problem) => {
-                    let mut buffer = String::new();
-                    let writer = unsafe { BufWriter::new(buffer.as_mut_vec()) };
-                    serialize_problem(writer, &problem).unwrap();
-                    let problem = CString::new(buffer.as_bytes()).unwrap();
+                    let mut writer = BufWriter::new(Vec::new());
+                    serialize_problem(&problem, &mut writer).unwrap();
+                    let bytes = writer.into_inner().expect("cannot use writer");
+                    let problem = CString::new(bytes).unwrap();
 
                     success(problem.as_ptr());
                 }
@@ -351,11 +353,14 @@ mod py_interop {
     fn convert_to_pragmatic(format: &str, inputs: Vec<String>) -> PyResult<String> {
         let readers = inputs.iter().map(|p| BufReader::new(p.as_bytes())).collect::<Vec<_>>();
         import_problem(format, Some(readers))
-            .map(|problem| {
-                let mut buffer = String::new();
-                serialize_problem(unsafe { BufWriter::new(buffer.as_mut_vec()) }, &problem).unwrap();
+            .and_then(|problem| {
+                let mut writer = BufWriter::new(Vec::new());
+                serialize_problem(&problem, &mut writer).unwrap();
 
-                buffer
+                writer
+                    .into_inner()
+                    .map_err(|err| format!("BufWriter: {err}"))
+                    .and_then(|bytes| String::from_utf8(bytes).map_err(|err| format!("StringUTF8: {err}")))
             })
             .map_err(PyOSError::new_err)
     }
@@ -458,11 +463,13 @@ mod wasm {
 
         match import_problem(format, Some(readers)) {
             Ok(problem) => {
-                let mut buffer = String::new();
-                let writer = unsafe { BufWriter::new(buffer.as_mut_vec()) };
-                serialize_problem(writer, &problem).unwrap();
+                let mut writer = BufWriter::new(Vec::new());
+                serialize_problem(&problem, &mut writer).unwrap();
+
+                let bytes = writer.into_inner().unwrap();
+                let result = String::from_utf8(bytes).unwrap();
 
-                Ok(JsValue::from_str(buffer.as_str()))
+                Ok(JsValue::from_str(result.as_str()))
             }
             Err(err) => Err(JsValue::from_str(err.to_string().as_str())),
         }

vrp-pragmatic/src/format/problem/model.rs

@@ -715,6 +715,6 @@ pub fn deserialize_locations<R: Read>(reader: BufReader<R>) -> Result<Vec<Locati
 }
 
 /// Serializes `problem` in json from `writer`.
-pub fn serialize_problem<W: Write>(writer: BufWriter<W>, problem: &Problem) -> Result<(), Error> {
+pub fn serialize_problem<W: Write>(problem: &Problem, writer: &mut BufWriter<W>) -> Result<(), Error> {
     serde_json::to_writer_pretty(writer, problem).map_err(Error::from)
 }

vrp-pragmatic/src/format/solution/geo_serializer.rs

@@ -84,8 +84,8 @@ pub fn serialize_solution_as_geojson<W: Write>(
 
 /// Serializes named location list with their color index.
 pub fn serialize_named_locations_as_geojson<W: Write>(
-    writer: BufWriter<W>,
     locations: &[(String, Location, usize)],
+    writer: &mut BufWriter<W>,
 ) -> Result<(), Error> {
     let geo_json = create_geojson_named_locations(locations)?;