Merge pull request #58 from urbit/as/build-ci

Change CI to enforce compiler warnings as errors

.github/workflows/ares-once.yml

@@ -0,0 +1,8 @@
+name: 'Ares: On Demand'
+
+on:
+  workflow_dispatch
+
+jobs:
+  urbit:
+    uses: ./.github/workflows/ares-shared.yml

.github/workflows/ares-shared.yml

@@ -10,21 +10,33 @@ defaults:
 
 jobs:
   build:
-    strategy:
-      matrix:
-        os: [ 'ubuntu-latest', 'macos-latest' ]
-    runs-on: ${{ matrix.os }}
+    runs-on: 'ubuntu-latest'
     steps:
       - uses: actions/checkout@v3
 
+      # Makes it easier to determine why CI might differ from local linter/build
+      #
+      - name: Version
+        run: rustc --version
+
       - name: Format
         run: cargo fmt --check
 
+      # See clippy linter docs: https://github.com/rust-lang/rust-clippy
+      #
+      # First linter is set to fail for all warnings, then ignored warnings are
+      # explicitly listed
+      #
       - name: Lint
-        run: cargo clippy -- --deny warnings --allow clippy::missing_safety_doc
+        run: |
+          cargo clippy \
+            --all-targets \
+            --no-deps \
+            -- -D warnings \
+            -A clippy::missing_safety_doc
 
       - name: Build
-        run: cargo build --verbose
+        run: cargo build --release --verbose
 
       - name: Run tests
         run: cargo test --verbose

rust/ares/build.rs

@@ -1,11 +1,58 @@
 fn main() {
+    use std::env;
+    let profile = env::var("PROFILE").unwrap();
+
     println!("cargo:rerun-if-changed=build.rs");
     println!("cargo:rerun-if-changed=./src/pma");
+
+    match profile.as_ref() {
+        "debug" => debug(),
+        "release" => release(),
+        _ => {
+            println!("unknown profile: {}", profile);
+            std::process::exit(-1);
+        }
+    }
+}
+
+fn debug() {
     cc::Build::new()
         .file("./src/pma/malloc.c")
         .file("./src/pma/includes/checksum.c")
-        // .opt_level(3)
         .opt_level(0)
         .flag("-g3")
+        .flag("-Wall")
+        .flag("-Wextra")
+        .flag("-Wpedantic")
+        .flag("-Wformat=2")
+        .flag("-Wno-unused-parameter")
+        .flag("-Wshadow")
+        .flag("-Wwrite-strings")
+        .flag("-Wstrict-prototypes")
+        .flag("-Wold-style-definition")
+        .flag("-Wredundant-decls")
+        .flag("-Wnested-externs")
+        .flag("-Wmissing-include-dirs")
+        .compile("pma_malloc");
+}
+
+fn release() {
+    cc::Build::new()
+        .file("./src/pma/malloc.c")
+        .file("./src/pma/includes/checksum.c")
+        .warnings_into_errors(true)
+        .opt_level(3)
+        .flag("-Wall")
+        .flag("-Wextra")
+        .flag("-Wpedantic")
+        .flag("-Wformat=2")
+        .flag("-Wno-unused-parameter")
+        .flag("-Wshadow")
+        .flag("-Wwrite-strings")
+        .flag("-Wstrict-prototypes")
+        .flag("-Wold-style-definition")
+        .flag("-Wredundant-decls")
+        .flag("-Wnested-externs")
+        .flag("-Wmissing-include-dirs")
         .compile("pma_malloc");
 }

rust/ares/src/jets/math.rs

@@ -741,7 +741,7 @@ mod tests {
 
     #[allow(non_snake_case)]
     fn A(stack: &mut NockStack, ubig: &UBig) -> Noun {
-        Atom::from_ubig(stack, &ubig).as_noun()
+        Atom::from_ubig(stack, ubig).as_noun()
     }
 
     fn assert_noun_eq(stack: &mut NockStack, mut a: Noun, mut b: Noun) {
@@ -817,7 +817,7 @@ mod tests {
 
     #[test]
     fn test_met() {
-        let ref mut s = init();
+        let s = &mut init();
         let a = atom_128(s).as_atom().unwrap();
         assert_eq!(met(0, a), 128);
         assert_eq!(met(1, a), 64);
@@ -842,7 +842,7 @@ mod tests {
 
     #[test]
     fn test_dec() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, _a24, a63, _a96, a128) = atoms(s);
         assert_jet_ubig(s, jet_dec, a128, ubig!(0xdeadbeef12345678fedcba987654320f));
         assert_jet(s, jet_dec, a63, D(0x7ffffffffffffffe));
@@ -851,7 +851,7 @@ mod tests {
 
     #[test]
     fn test_add() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet(
             s,
             jet_add,
@@ -869,7 +869,7 @@ mod tests {
 
     #[test]
     fn test_sub() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet(
             s,
             jet_sub,
@@ -889,7 +889,7 @@ mod tests {
 
     #[test]
     fn test_mul() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet(
             s,
             jet_mul,
@@ -913,7 +913,7 @@ mod tests {
 
     #[test]
     fn test_div() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet(s, jet_div, &[atom_128, atom_96], ubig!(0xe349f8f0));
         assert_math_jet(s, jet_div, &[atom_96, atom_63], ubig!(0x1f59d6018));
         assert_math_jet(s, jet_div, &[atom_63, atom_96], ubig!(0));
@@ -935,7 +935,7 @@ mod tests {
 
     #[test]
     fn test_mod() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet(
             s,
             jet_mod,
@@ -954,7 +954,7 @@ mod tests {
 
     #[test]
     fn test_dvr() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, a63, a96, a128) = atoms(s);
         let a264 = atom_264(s);
         let a528 = atom_528(s);
@@ -999,7 +999,7 @@ mod tests {
 
     #[test]
     fn test_lth() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet_noun(s, jet_lth, &[atom_128, atom_96], NO);
         assert_math_jet_noun(s, jet_lth, &[atom_96, atom_63], NO);
         assert_math_jet_noun(s, jet_lth, &[atom_63, atom_96], YES);
@@ -1012,7 +1012,7 @@ mod tests {
 
     #[test]
     fn test_lte() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet_noun(s, jet_lte, &[atom_128, atom_96], NO);
         assert_math_jet_noun(s, jet_lte, &[atom_96, atom_63], NO);
         assert_math_jet_noun(s, jet_lte, &[atom_63, atom_96], YES);
@@ -1025,7 +1025,7 @@ mod tests {
 
     #[test]
     fn test_gth() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet_noun(s, jet_gth, &[atom_128, atom_96], YES);
         assert_math_jet_noun(s, jet_gth, &[atom_96, atom_63], YES);
         assert_math_jet_noun(s, jet_gth, &[atom_63, atom_96], NO);
@@ -1038,7 +1038,7 @@ mod tests {
 
     #[test]
     fn test_gte() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_math_jet_noun(s, jet_gte, &[atom_128, atom_96], YES);
         assert_math_jet_noun(s, jet_gte, &[atom_96, atom_63], YES);
         assert_math_jet_noun(s, jet_gte, &[atom_63, atom_96], NO);
@@ -1051,7 +1051,7 @@ mod tests {
 
     #[test]
     fn test_bex() {
-        let ref mut s = init();
+        let s = &mut init();
         assert_jet(s, jet_bex, D(0), D(1));
         assert_jet(s, jet_bex, D(5), D(32));
         assert_jet(s, jet_bex, D(62), D(0x4000000000000000));
@@ -1065,7 +1065,7 @@ mod tests {
 
     #[test]
     fn test_lsh() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, _a63, a96, a128) = atoms(s);
         let sam = T(s, &[a0, a24]);
         assert_jet(s, jet_lsh, sam, D(0x10eca86));
@@ -1095,7 +1095,7 @@ mod tests {
 
     #[test]
     fn test_rsh() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, _a63, a96, a128) = atoms(s);
         let sam = T(s, &[a0, a24]);
         assert_jet(s, jet_rsh, sam, D(0x43b2a1));
@@ -1120,7 +1120,7 @@ mod tests {
 
     #[test]
     fn test_con() {
-        let ref mut s = init();
+        let s = &mut init();
         let (_a0, _a24, a63, _a96, a128) = atoms(s);
         assert_math_jet(s, jet_con, &[atom_0, atom_0], ubig!(0));
         assert_math_jet(
@@ -1142,7 +1142,7 @@ mod tests {
 
     #[test]
     fn test_dis() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, _a63, _a96, _a128) = atoms(s);
         assert_math_jet(s, jet_dis, &[atom_0, atom_0], ubig!(0));
         assert_math_jet(s, jet_dis, &[atom_24, atom_96], ubig!(0x22442));
@@ -1159,7 +1159,7 @@ mod tests {
 
     #[test]
     fn test_mix() {
-        let ref mut s = init();
+        let s = &mut init();
         let (_a0, _a24, _a63, _a96, a128) = atoms(s);
         assert_math_jet(s, jet_mix, &[atom_0, atom_0], ubig!(0));
         assert_math_jet(
@@ -1181,7 +1181,7 @@ mod tests {
 
     #[test]
     fn test_end() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, _a63, a96, a128) = atoms(s);
         let sam = T(s, &[a0, a24]);
         assert_jet(s, jet_end, sam, D(0x1));
@@ -1203,7 +1203,7 @@ mod tests {
 
     #[test]
     fn test_cat() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, a63, _a96, a128) = atoms(s);
         let sam = T(s, &[a0, a0, a0]);
         assert_jet(s, jet_cat, sam, D(0));
@@ -1223,7 +1223,7 @@ mod tests {
 
     #[test]
     fn test_cut() {
-        let ref mut s = init();
+        let s = &mut init();
         let (_a0, a24, _a63, a96, a128) = atoms(s);
         let run = T(s, &[D(0), D(5)]);
         let sam = T(s, &[D(0), run, a24]);
@@ -1242,7 +1242,7 @@ mod tests {
 
     #[test]
     fn test_can() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, a63, a96, a128) = atoms(s);
         let sam = T(s, &[D(0), D(0)]);
         assert_jet(s, jet_can, sam, D(0));
@@ -1261,7 +1261,7 @@ mod tests {
 
     #[test]
     fn test_rep() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, a63, a96, a128) = atoms(s);
         let sam = T(s, &[D(0), D(0)]);
         assert_jet(s, jet_rep, sam, D(0));
@@ -1273,7 +1273,7 @@ mod tests {
 
     #[test]
     fn test_rip() {
-        let ref mut s = init();
+        let s = &mut init();
         let (_a0, _a24, _a63, _a96, a128) = atoms(s);
         let sam = T(s, &[D(0), D(0)]);
         assert_jet(s, jet_rip, sam, D(0));
@@ -1296,7 +1296,7 @@ mod tests {
 
     #[test]
     fn test_jet_met() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, _a63, _a96, a128) = atoms(s);
         let sam = T(s, &[a0, a0]);
         assert_jet(s, jet_met, sam, D(0));
@@ -1310,7 +1310,7 @@ mod tests {
 
     #[test]
     fn test_mug() {
-        let ref mut s = init();
+        let s = &mut init();
         let (a0, a24, a63, a96, a128) = atoms(s);
         assert_jet(s, jet_mug, a0, D(0x79ff04e8));
         assert_jet(s, jet_mug, a24, D(0x69d59d90));
@@ -1331,8 +1331,8 @@ mod tests {
 
     #[test]
     fn test_rev() {
-        let ref mut s = init();
-        let (_a0, a24, _a63, a96, a128) = atoms(s);
+        let s = &mut init();
+        let (_a0, a24, _a63, _a96, _a128) = atoms(s);
         let sam = T(s, &[D(0), D(60), a24]);
         assert_jet(s, jet_rev, sam, D(0xc2a6e1000000000));
         let test = 0x1234567890123u64;

rust/ares/src/pma/malloc.c

@@ -246,7 +246,6 @@
 /**
  * Page statuses used in page directory
  */
-typedef enum PMAPageStatus PMAPageStatus;
 enum PMAPageStatus {
   UNALLOCATED,
   FREE,
@@ -254,6 +253,7 @@ enum PMAPageStatus {
   FIRST,
   FOLLOW
 };
+typedef enum PMAPageStatus PMAPageStatus;
 
 /**
  * Directory entry for a page in virtual memory
@@ -436,7 +436,7 @@ struct PMAState {
 PMAState *_pma_state = NULL;
 
 void
-pma_state_free()
+pma_state_free(void)
 {
   if (_pma_state->metadata) free(_pma_state->metadata);
   free(_pma_state);
@@ -444,7 +444,7 @@ pma_state_free()
 }
 
 int
-pma_state_malloc()
+pma_state_malloc(void)
 {
   if (_pma_state != NULL) return 1;
   PMAState *ret = calloc(1, sizeof *ret);
@@ -792,8 +792,9 @@ pma_load(const char *path) {
   //
 
   // Read magic code
-  read(snapshot_fd, &_pma_state->metadata->magic_code, sizeof(uint64_t));
-  if (_pma_state->metadata->magic_code != PMA_MAGIC_CODE) {
+  if (-1 == read(snapshot_fd, &_pma_state->metadata->magic_code, sizeof(uint64_t))) {
+    LOAD_ERROR;
+  } else if (_pma_state->metadata->magic_code != PMA_MAGIC_CODE) {
     errno = EILSEQ;
     LOAD_ERROR;
   }
@@ -1138,6 +1139,12 @@ pma_sync(uint64_t epoch, uint64_t event, uint64_t root) {
     = crc_32((unsigned char *)_pma_state->metadata, PMA_PAGE_SIZE);
 
   // Sync metadata
+  // 
+  // Note:  It's a long-standing Unix convention that while both write and
+  //        pwrite return the number of bytes written, when operating on a file
+  //        (as opposed to a pipe or socket) it is assumed that the entire
+  //        buffer will be written. If this isn't the case, an error has
+  //        occurred.
   bytes_out = pwrite(
       _pma_state->snapshot_fd,
       _pma_state->metadata,
@@ -1604,7 +1611,7 @@ _pma_get_cached_pages(uint64_t num_pages) {
     // If run larger than necessary by two pages...
     if (valid_page_run->length > (num_pages + 1)) {
       // Reduce it
-      valid_page_run->page += (num_pages * PMA_PAGE_SIZE);
+      valid_page_run->page = (uint8_t*)valid_page_run->page + (num_pages * PMA_PAGE_SIZE);
       valid_page_run->length -= num_pages;
 
     // Otherwise...
@@ -1675,7 +1682,7 @@ _pma_get_new_page(PMAPageStatus status) {
   assert(address == _pma_state->metadata->arena_end);
 
   // Record PMA expansion
-  _pma_state->metadata->arena_end += PMA_PAGE_SIZE;
+  _pma_state->metadata->arena_end = (uint8_t*)_pma_state->metadata->arena_end + PMA_PAGE_SIZE;
 
   // Add page to dirty list
   _pma_mark_page_dirty(PTR_TO_INDEX(address), offset, status, 1);
@@ -1728,7 +1735,7 @@ _pma_get_new_pages(uint64_t num_pages) {
 
   // Update offset of next open dpage
   _pma_state->metadata->next_offset += bytes;
-  _pma_state->metadata->arena_end += bytes;
+  _pma_state->metadata->arena_end = (uint8_t*)_pma_state->metadata->arena_end + bytes;
 
   // Add allocated pages to dirty list
   _pma_mark_page_dirty(PTR_TO_INDEX(address), offset, FIRST, num_pages);

rust/ares/src/snapshot/pma.rs

@@ -123,7 +123,7 @@ mod tests {
 
         unsafe {
             pma_init(path);
-            let ref mut stack = NockStack::new(8 << 10 << 10, 0);
+            let stack = &mut NockStack::new(8 << 10 << 10, 0);
             let root = IndirectAtom::new_raw(stack, 1, &0xffff_ffff_ffff_ffff).as_noun();
             let eight = pma_malloc(8) as *mut u64;
             *eight = 0xdeadbeef;