Merge pull request #78 from omnibor/abrinker/improvements

Miscellaneous improvements

Author: alilleybrinker

Cargo.toml

@@ -1,4 +1,5 @@
 [workspace]
 
 members = ["omnibor", "gitoid"]
+resolver = "2"
 

gitoid/Cargo.toml

@@ -15,6 +15,6 @@ crate-type = ["rlib", "cdylib"]
 
 [dependencies]
 hex = "0.4.3"
-sha1 = "0.10.4"
-sha2 = "0.10.5"
-url = "2.2.2"
+sha1 = "0.10.6"
+sha2 = "0.10.8"
+url = "2.4.1"

gitoid/Makefile

@@ -4,7 +4,7 @@
 # It is intended to be easy for newcomers to the project to understand.
 
 # Variables for cbindgen.
-CBINDGEN_CONFIG="../cbindgen.toml"
+CBINDGEN_CONFIG="./cbindgen.toml"
 
 # Variables for the header file.
 INCLUDE_DIR="./include"

cbindgen.toml renamed to gitoid/cbindgen.toml

@@ -2,7 +2,7 @@
 language = "C"
 
 # Wrap generated file in an include guard.
-include_guard = "gitbom_h"
+include_guard = "gitoid_h"
 
 # Note the version of cbindgen used to generate the header.
 include_version = true
@@ -20,7 +20,7 @@ style = "both"
 header = """
 /**
  * @file
- * @brief "GitBom"
+ * @brief "GitOID"
  */
 """
 
@@ -40,23 +40,3 @@ autogen_warning = "/* Warning, this file is autogenerated by cbindgen. Don't mod
 
 # Prefix enum variants with the name of the overall type.
 prefix_with_name = true
-
-
-#==============================================================================
-# Parsing Rules
-#------------------------------------------------------------------------------
-
-[parse]
-
-# Parse dependencies.
-#parse_deps = true
-#include = []
-
-#==============================================================================
-# Macro Expansion Rules
-#------------------------------------------------------------------------------
-
-#[parse.expand]
-
-# Expand macros for `gitoid`.
-#crates = ["gitoid"]

gitoid/src/ffi/gitoid.rs

@@ -15,8 +15,14 @@ use std::ffi::CStr;
 use std::ffi::CString;
 use std::fs::File;
 use std::io::BufReader;
+#[cfg(target_family = "unix")]
 use std::os::unix::prelude::FromRawFd;
+#[cfg(target_family = "unix")]
 use std::os::unix::prelude::RawFd;
+#[cfg(target_family = "windows")]
+use std::os::windows::io::FromRawHandle;
+#[cfg(target_family = "windows")]
+use std::os::windows::prelude::RawHandle;
 use std::ptr::null;
 use std::ptr::null_mut;
 use std::slice;
@@ -64,6 +70,8 @@ pub extern "C" fn gitoid_invalid(gitoid: *const GitOid) -> c_int {
 
 /// Construct a new `GitOid` from a buffer of bytes.
 ///
+/// `content_len` is the number of elements, not the number of bytes.
+///
 /// `content_len` times 8 (byte size) must be less than or equal to the
 /// maximum size representable with an unsigned integer at the size used by
 /// the ISA (32-bit or 64-bit usually).
@@ -74,7 +82,7 @@ pub extern "C" fn gitoid_invalid(gitoid: *const GitOid) -> c_int {
 pub extern "C" fn gitoid_new_from_bytes(
     hash_algorithm: HashAlgorithm,
     object_type: ObjectType,
-    content: *const u8,
+    content: *mut u8,
     content_len: usize,
 ) -> GitOid {
     let output = catch_panic(|| {
@@ -277,5 +285,5 @@ pub extern "C" fn gitoid_str_free(s: *mut c_char) {
         return;
     }
 
-    unsafe { CString::from_raw(s) };
+    let _ = unsafe { CString::from_raw(s) };
 }

gitoid/src/gitoid.rs

@@ -55,7 +55,7 @@ impl GitOid {
     ///
     /// This construction should _only_ be used for error-handling purposes
     /// when using the `gitoid` crate over FFI.
-    pub fn new_invalid() -> Self {
+    pub(crate) fn new_invalid() -> Self {
         GitOid {
             hash_algorithm: HashAlgorithm::Sha1,
             object_type: ObjectType::Blob,

gitoid/test/.gitignore

@@ -0,0 +1,2 @@
+c_test.exe
+c_test

gitoid/test/c/test.c

@@ -5,76 +5,70 @@
 #include "gitoid.h"
 
 #define LEN(arr) (sizeof(arr) / sizeof(arr[0]));
+#define TEST(NAME) {.name = #NAME, .fn = NAME}
 
 void test_gitoid_new_from_str() {
     GitOid gitoid = gitoid_new_from_str(HashAlgorithm_Sha1, ObjectType_Blob, "hello world"); 
+    assert(!gitoid_invalid(&gitoid));
     assert(gitoid.len == 20);
     assert(gitoid.value[0] == 149);
 }
 
 void test_gitoid_new_from_bytes() {
-    // Section that creates the byte array was heavily inspired by [1].
-    //
-    // Does not do error checking, and is intended solely for test purposes.
-    // The length of `byte_array` is equal to the length of `string` plus one,
-    // to make space for the nul-terminator.
-    //
-    // [1]: https://stackoverflow.com/a/3409211/2308264
-    const char *string = "hello_world";
-    const char *position = string;
-    unsigned char byte_array[12];
-    size_t size = LEN(byte_array);
-    for (size_t count = 0; count < size; ++count) {
-        sscanf(position, "%2hhx", &byte_array[count]);
-        position += 2;
-    }
-    uint8_t byte_array_length = sizeof byte_array;
+    unsigned char bytes[] = {0x00, 0x01, 0x02, 0x03,
+                             0x04, 0x05, 0x06, 0x07,
+                             0x08, 0x09, 0x0A, 0x0B,
+                             0x0C, 0x0D, 0x0E, 0x0F};
+    uint8_t bytes_len = LEN(bytes);
 
     GitOid gitoid = gitoid_new_from_bytes(
         HashAlgorithm_Sha1,
         ObjectType_Blob,
-        &byte_array_length,
-        *byte_array
+        bytes,
+        bytes_len
     );
 
+    assert(!gitoid_invalid(&gitoid));
     assert(gitoid.len == 20);
-    assert(gitoid.value[0] == 130);
+    assert(gitoid.value[0] == 182);
 }
 
 void test_gitoid_new_from_url() {
     char *url = "gitoid:blob:sha256:fee53a18d32820613c0527aa79be5cb30173c823a9b448fa4817767cc84c6f03";
     GitOid gitoid = gitoid_new_from_url(url);
+    assert(!gitoid_invalid(&gitoid));
     assert(gitoid.len == 32);
     assert(gitoid.value[0] == 254);
 }
 
 void test_gitoid_get_url() {
     char *url_in = "gitoid:blob:sha256:fee53a18d32820613c0527aa79be5cb30173c823a9b448fa4817767cc84c6f03";
     GitOid gitoid = gitoid_new_from_url(url_in);
+    assert(!gitoid_invalid(&gitoid));
     char *url_out = gitoid_get_url(&gitoid);
     assert(strncmp(url_in, url_out, 83) == 0);
     gitoid_str_free(url_out);
 }
 
 void test_gitoid_hash_algorithm_name() {
     GitOid gitoid = gitoid_new_from_str(HashAlgorithm_Sha1, ObjectType_Blob, "hello world");
+    assert(!gitoid_invalid(&gitoid));
     const char *hash_algorithm = gitoid_hash_algorithm_name(gitoid.hash_algorithm);
     assert(strncmp(hash_algorithm, "sha1", 4) == 0);
 }
 
 void test_gitoid_object_type_name() {
     GitOid gitoid = gitoid_new_from_str(HashAlgorithm_Sha1, ObjectType_Blob, "hello world");
+    assert(!gitoid_invalid(&gitoid));
     const char *object_type = gitoid_object_type_name(gitoid.object_type);
     assert(strncmp(object_type, "blob", 4) == 0);
 }
 
 void test_gitoid_validity() {
-    // Notice the SHA type is invalid.
     char *validity_url = "gitoid:blob:sha000:fee53a18d32820613c0527aa79be5cb30173c823a9b448fa4817767cc84c6f03";
     GitOid gitoid = gitoid_new_from_url(validity_url);
     assert(gitoid_invalid(&gitoid));
 
-    // Also test the error message reporting.
     char *expected_msg = "string is not a valid GitOID URL";
     char error_msg[256];
     gitoid_get_error_message(error_msg, 256);
@@ -91,14 +85,14 @@ typedef struct test {
 int main() {
     setvbuf(stdout, NULL, _IONBF, 0);
 
-    test_t tests[7] = {
-        {.name = "gitoid_new_from_str", .fn = test_gitoid_new_from_str},
-        {.name = "gitoid_new_from_bytes", .fn = test_gitoid_new_from_bytes},
-        {.name = "gitoid_new_from_url", .fn = test_gitoid_new_from_url},
-        {.name = "gitoid_get_url", .fn = test_gitoid_get_url},
-        {.name = "gitoid_hash_algorithm_name", .fn = test_gitoid_hash_algorithm_name},
-        {.name = "gitoid_object_type_name", .fn = test_gitoid_object_type_name},
-        {.name = "gitoid_validity", .fn = test_gitoid_validity},
+    test_t tests[] = {
+        TEST(test_gitoid_new_from_str),
+        TEST(test_gitoid_new_from_bytes),
+        TEST(test_gitoid_new_from_url),
+        TEST(test_gitoid_get_url),
+        TEST(test_gitoid_hash_algorithm_name),
+        TEST(test_gitoid_object_type_name),
+        TEST(test_gitoid_validity),
     };
 
     size_t n_tests = LEN(tests);

omnibor/Cargo.toml

@@ -12,7 +12,3 @@ version = "0.1.7"
 
 [dependencies]
 gitoid = "0.1.5"
-hex = "0.4.3"
-im = "15.1.0"
-sha1 = "0.10.4"
-sha2 = "0.10.5"

omnibor/src/lib.rs

@@ -1,120 +1,2 @@
-use gitoid::GitOid;
-use im::{HashSet, Vector};
 
-/// A [persistent][wiki] collection of [git oids][git_scm].
-///
-/// Why persistent? While Rust and the borrow checker is great about ownership and
-/// mutation, always knowing that a Ref will not change if passed as a parameter
-/// to a function eliminates a class of errors.
-///
-/// [wiki]: https://en.wikipedia.org/wiki/Persistent_data_structure
-/// [git_scm]: https://git-scm.com/book/en/v2/Git-Internals-Git-Objects
-#[derive(Clone, PartialOrd, Eq, Ord, Debug, Hash, PartialEq)]
-pub struct OmniBor {
-    git_oids: HashSet<GitOid>,
-}
-
-impl FromIterator<GitOid> for OmniBor {
-    /// Create an OmniBor from many GitOids
-    fn from_iter<T>(gitoids: T) -> Self
-    where
-        T: IntoIterator<Item = GitOid>,
-    {
-        let me = OmniBor::new();
-        me.add_many(gitoids)
-    }
-}
-
-impl OmniBor {
-    /// Create a new instance
-    #[allow(clippy::new_without_default)]
-    pub fn new() -> Self {
-        Self {
-            git_oids: HashSet::new(),
-        }
-    }
-
-    /// Create a OmniBor from many GitOids
-    pub fn new_from_iterator<I>(gitoids: I) -> Self
-    where
-        I: IntoIterator<Item = GitOid>,
-    {
-        let me = OmniBor::new();
-        me.add_many(gitoids)
-    }
-
-    /// Add a `GitOid` hash to the `OmniBor`.
-    ///
-    /// Note that this creates a new persistent data structure under the hood.
-    pub fn add(&self, gitoid: GitOid) -> Self {
-        self.add_many([gitoid])
-    }
-
-    /// Append many `GitOid`s and return a new `OmniBor`
-    pub fn add_many<I>(&self, gitoids: I) -> Self
-    where
-        I: IntoIterator<Item = GitOid>,
-    {
-        let mut updated = self.git_oids.clone(); // im::HashSet has O(1) cloning
-        for gitoid in gitoids {
-            updated = updated.update(gitoid);
-        }
-        Self { git_oids: updated }
-    }
-
-    /// Return the `Vector` of `GitOid`s.
-    pub fn get_oids(&self) -> HashSet<GitOid> {
-        self.git_oids.clone() // im::HashSet as O(1) cloning.
-    }
-
-    /// Get a sorted `Vector` of `GitOid`s.
-    ///
-    /// In some cases, getting a sorted `Vector` of oids is desirable.
-    /// This function (cost O(n log n)) returns a `Vector` of sorted oids
-    pub fn get_sorted_oids(&self) -> Vector<GitOid> {
-        let mut ret = self.git_oids.clone().into_iter().collect::<Vector<_>>();
-        ret.sort();
-        ret
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use gitoid::{GitOid, HashAlgorithm, ObjectType::Blob};
-    use im::vector;
-
-    use super::*;
-
-    #[test]
-    fn test_add() {
-        let oid = GitOid::new_from_str(HashAlgorithm::Sha256, Blob, "Hello");
-        assert_eq!(OmniBor::new().add(oid).get_sorted_oids(), vector![oid])
-    }
-
-    #[test]
-    fn test_add_many() {
-        let mut oids: Vector<GitOid> = vec!["eee", "Hello", "Cat", "Dog"]
-            .into_iter()
-            .map(|s| GitOid::new_from_str(HashAlgorithm::Sha256, Blob, s))
-            .collect();
-
-        let da_bom = OmniBor::new().add_many(oids.clone());
-        oids.sort();
-        assert_eq!(da_bom.get_sorted_oids(), oids);
-    }
-
-    #[test]
-    fn test_add_gitoid_to_gitbom() {
-        let input = "hello world".as_bytes();
-
-        let generated_gitoid = GitOid::new_from_bytes(HashAlgorithm::Sha256, Blob, input);
-
-        let new_gitbom = OmniBor::new();
-        let new_gitbom = new_gitbom.add(generated_gitoid);
-
-        assert_eq!(
-            "fee53a18d32820613c0527aa79be5cb30173c823a9b448fa4817767cc84c6f03",
-            new_gitbom.get_sorted_oids()[0].hash().as_hex()
-        )
-    }
-}
+// TODO(abrinker): Complete first draft of the rewrite of this crate.