Many fixes (#51)

* Added fullstops * Improved warning message for decode mode * Updated changelog * Version bump * Fixed #50 * Fixed #49 * Updated changelog * Fixed get_ref_block_choice macro * Fixed get_burst_or_guess! macro * Fixed get_burst_or_guess * Updated changelog * Fixed test suite
darrenldl · Mar 24, 2018 · 95e136f · 95e136f
1 parent 3036c15
commit 95e136f
Show file tree

Hide file tree

Showing 16 changed files with 99 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,13 @@
+## 1.0.3
+- General output text polishing
+- Fixed container size calculation for when --no-meta flag is supplied
+- Fixed encode mode for when --no-meta flag is supplied
+  - Previously rsbx would leave a blank spot at where the metadata block would otherwise sit instead of skipping the metadata block properly
+  - SBX containers created with --no-meta flag enabled prior to this fix are still valid and can be decoded by rsbx successfully
+- Fixed reference block retrieval procedure related code
+  - Previously for decode, sort, and check mode, rsbx would interpret --no-meta flag incorrecty for reference block preference, namely any block type is allowed when the flag is absent, and metadata block is preferred when the flag is present, while it should be the other way around
+- Fixed a crash that occurs when sort mode is used with a SBX container of RS enabled version, and using data block as reference block
+
 ## 1.0.2
 - Fixed wording of error correction parameters interpretation strings in calc mode
 - Fixed container size calculation for when file size is 0

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name    = "rsbx"
-version = "1.0.2"
+version = "1.0.3"
 authors = ["Darren Ldl <darrenldldev@gmail.com>"]
 build   = "build.rs"
 exclude = [

diff --git a/src/block_utils.rs b/src/block_utils.rs
@@ -282,6 +282,7 @@ pub fn guess_burst_err_resistance_level(in_file       : &str,
         for index in 0..seq_nums.len() {
             let expected_seq_num =
                 sbx_block::calc_seq_num_at_index(index as u64,
+                                                 None,
                                                  Some((data_shards,
                                                        parity_shards,
                                                        level)));

diff --git a/src/cli_calc.rs b/src/cli_calc.rs
@@ -30,7 +30,9 @@ This means this option has no effect for version 17, 18, 19."))
 }
 
 pub fn calc<'a>(matches : &ArgMatches<'a>) -> i32 {
-    let version   = get_version!(matches);
+    let version = get_version!(matches);
+
+    let meta_enabled = Some(get_meta_enabled!(matches));
 
     let in_file_size =
         match u64::from_str(matches.value_of("in_file_size").unwrap()) {
@@ -55,11 +57,13 @@ pub fn calc<'a>(matches : &ArgMatches<'a>) -> i32 {
 
     let out_file_size =
         file_utils::from_orig_file_size::calc_container_size(version,
+                                                             meta_enabled,
                                                              data_par_burst,
                                                              in_file_size);
 
     let total_block_count =
         file_utils::from_orig_file_size::calc_total_block_count_exc_burst_gaps(version,
+                                                                               meta_enabled,
                                                                                data_par_burst,
                                                                                in_file_size);
 
@@ -134,7 +138,7 @@ pub fn calc<'a>(matches : &ArgMatches<'a>) -> i32 {
                     "    Warning :";
                     "";
                     "        Burst error resistance level of {} may not provide", burst;
-                    "        meaningful resistance";
+                    "        meaningful resistance.";
                     "";
                 );
             }

diff --git a/src/cli_encode.rs b/src/cli_encode.rs
@@ -112,6 +112,8 @@ pub fn encode<'a>(matches : &ArgMatches<'a>) -> i32 {
 
     let pr_verbosity_level = get_pr_verbosity_level!(matches);
 
+    let meta_enabled = get_meta_enabled!(matches);
+
     if matches.is_present("info_only") {
         let in_file_meta  = match file_utils::get_file_metadata(in_file) {
             Ok(x)  => x,
@@ -140,6 +142,7 @@ pub fn encode<'a>(matches : &ArgMatches<'a>) -> i32 {
 
         let out_file_size =
             file_utils::from_orig_file_size::calc_container_size(version,
+                                                                 Some(meta_enabled),
                                                                  data_par_burst,
                                                                  in_file_size);
 
@@ -175,7 +178,7 @@ pub fn encode<'a>(matches : &ArgMatches<'a>) -> i32 {
         let param = Param::new(version,
                                &uid,
                                data_par_burst,
-                               matches.is_present("no_meta"),
+                               meta_enabled,
                                hash_type,
                                in_file,
                                &out,

diff --git a/src/cli_macros.rs b/src/cli_macros.rs
@@ -272,9 +272,17 @@ macro_rules! get_ref_block_choice {
         use sbx_block::BlockType;
 
         if $matches.is_present("no_meta") {
-            Prefer(BlockType::Meta)
-        } else {
             Any
+        } else {
+            Prefer(BlockType::Meta)
         }
     }}
 }
+
+macro_rules! get_meta_enabled {
+    (
+        $matches:expr
+    ) => {{
+        !$matches.is_present("no_meta")
+    }}
+}
diff --git a/src/decode_core.rs b/src/decode_core.rs
@@ -354,7 +354,10 @@ pub fn decode(param           : &Param,
     } else {
         print_block!(
             "";
-            "Warning : Reference block is not a metadata block, output file may contain data padding";
+            "Warning :";
+            "";
+            "    Reference block is not a metadata block, output file";
+            "    may contain data padding.";
             "";)
     }
 

diff --git a/src/encode_core.rs b/src/encode_core.rs
@@ -137,7 +137,7 @@ impl Param {
     pub fn new(version            : Version,
                uid                : &[u8; SBX_FILE_UID_LEN],
                data_par_burst     : Option<(usize, usize, usize)>,
-               no_meta            : bool,
+               meta_enabled       : bool,
                hash_type          : multihash::HashType,
                in_file            : &str,
                out_file           : &str,
@@ -147,7 +147,7 @@ impl Param {
             uid            : uid.clone(),
             data_par_burst,
             rs_enabled     : ver_uses_rs(version),
-            meta_enabled   : ver_forces_meta_enabled(version) || !no_meta,
+            meta_enabled   : ver_forces_meta_enabled(version) || meta_enabled,
             hash_type,
             in_file        : String::from(in_file),
             out_file       : String::from(out_file),
@@ -279,6 +279,7 @@ fn write_data_block(param  : &Param,
     let write_pos =
         calc_data_block_write_pos(param.version,
                                   block.get_seq_num(),
+                                  Some(param.meta_enabled),
                                   param.data_par_burst);
 
     block_sync_and_write(block,

diff --git a/src/file_utils.rs b/src/file_utils.rs
@@ -102,6 +102,7 @@ pub mod from_orig_file_size {
     }
 
     pub fn calc_total_block_count_exc_burst_gaps(version        : Version,
+                                                 meta_enabled   : Option<bool>,
                                                  data_par_burst : Option<(usize, usize, usize)>,
                                                  size           : u64)
                                                  -> u64 {
@@ -112,7 +113,15 @@ pub mod from_orig_file_size {
 
         match data_par_burst {
             None                 => {
-                let meta_block_count = 1;
+                let meta_enabled = meta_enabled.unwrap_or(true);
+
+                let meta_block_count =
+                    if meta_enabled {
+                        1
+                    } else {
+                        0
+                    };
+
                 let data_block_count =
                     calc_data_block_count_exc_burst_gaps(version,
                                                          data_par_burst,
@@ -131,6 +140,7 @@ pub mod from_orig_file_size {
     }
 
     pub fn calc_container_size(version        : Version,
+                               meta_enabled   : Option<bool>,
                                data_par_burst : Option<(usize, usize, usize)>,
                                size           : u64)
                                -> u64 {
@@ -140,6 +150,7 @@ pub mod from_orig_file_size {
             None                        => {
                 let block_count =
                     calc_total_block_count_exc_burst_gaps(version,
+                                                          meta_enabled,
                                                           data_par_burst,
                                                           size);
 
@@ -149,6 +160,7 @@ pub mod from_orig_file_size {
                 if burst == 0 {
                     let block_count =
                         calc_total_block_count_exc_burst_gaps(version,
+                                                              meta_enabled,
                                                               data_par_burst,
                                                               size);
 
@@ -192,6 +204,7 @@ pub mod from_orig_file_size {
                 while seq_num <= last_seq_num {
                     let index =
                         sbx_block::calc_data_block_write_index(seq_num,
+                                                               meta_enabled,
                                                                data_par_burst);
 
                     if index > last_index {

diff --git a/src/misc_macros.rs b/src/misc_macros.rs
@@ -124,6 +124,10 @@ macro_rules! get_burst_or_guess {
     ) => {{
         let burst = unwrap_or!($param.burst,
                                if ver_uses_rs($ref_block.get_version()) {
+                                   return_if_ref_not_meta!($ref_block_pos,
+                                                           $ref_block,
+                                                           "guess burst error resistance level");
+
                                    unwrap_or!(block_utils::guess_burst_err_resistance_level(&$param.in_file,
                                                                                             $ref_block_pos,
                                                                                             &$ref_block)?,
@@ -205,3 +209,12 @@ macro_rules! break_if_reached_required_len {
         }
     }}
 }
+
+macro_rules! shadow_to_avoid_use {
+    (
+        $var:ident
+    ) => {
+        #[allow(unused_variables)]
+        let $var = ();
+    }
+}
diff --git a/src/repair_core.rs b/src/repair_core.rs
@@ -233,6 +233,7 @@ pub fn repair_file(param : &Param)
         match ref_block.get_FSZ().unwrap() {
             Some(x) =>
                 calc_total_block_count_exc_burst_gaps(version,
+                                                      None,
                                                       data_par_burst,
                                                       x),
             None    => {
@@ -324,6 +325,7 @@ pub fn repair_file(param : &Param)
 
         let pos = sbx_block::calc_data_block_write_pos(version,
                                                        seq_num,
+                                                       None,
                                                        data_par_burst);
 
         reader.seek(SeekFrom::Start(pos))?;

diff --git a/src/rs_codec/repairer.rs b/src/rs_codec/repairer.rs
@@ -64,6 +64,7 @@ impl<'a> fmt::Display for RSRepairStats<'a> {
 
                     let index     =
                         sbx_block::calc_data_block_write_index(seq_num,
+                                                               None,
                                                                self.data_par_burst);
                     let block_pos = index * block_size;
 
@@ -246,6 +247,7 @@ impl RSRepairer {
                 if !self.buf_present[i] {
                     let pos = sbx_block::calc_data_block_write_pos(self.version,
                                                                    cur_seq_num,
+                                                                   None,
                                                                    self.data_par_burst);
                     repaired_blocks.push((pos, sbx_block::slice_buf(self.version,
                                                                     &self.buf[i])));

diff --git a/src/sbx_block/mod.rs b/src/sbx_block/mod.rs
@@ -263,17 +263,20 @@ pub fn calc_meta_block_all_write_indices(data_par_burst : Option<(usize, usize,
 
 pub fn calc_data_block_write_pos(version        : Version,
                                  seq_num        : u32,
+                                 meta_enabled   : Option<bool>,
                                  data_par_burst : Option<(usize, usize, usize)>)
                                  -> u64 {
     check_ver_consistent_with_opt!(version, data_par_burst);
 
     let block_size = ver_to_block_size(version) as u64;
 
     calc_data_block_write_index(seq_num,
+                                meta_enabled,
                                 data_par_burst) * block_size
 }
 
 pub fn calc_data_block_write_index(seq_num        : u32,
+                                   meta_enabled   : Option<bool>,
                                    data_par_burst : Option<(usize, usize, usize)>)
                                    -> u64 {
     // the following transforms seq num to data index
@@ -285,9 +288,14 @@ pub fn calc_data_block_write_index(seq_num        : u32,
 
     match data_par_burst {
         None                        => {
-            SBX_FIRST_DATA_SEQ_NUM as u64 + index
+            let meta_enabled = meta_enabled.unwrap_or(true);
+
+            if meta_enabled { SBX_FIRST_DATA_SEQ_NUM as u64 + index }
+            else            { index }
         },
         Some((data, parity, burst)) => {
+            shadow_to_avoid_use!(meta_enabled);
+
             if burst == 0 {
                 let meta_block_count = 1 + parity as u64;
 
@@ -407,13 +415,19 @@ pub fn calc_data_chunk_write_pos(version  : Version,
 }
 
 pub fn calc_seq_num_at_index(index          : u64,
+                             meta_enabled   : Option<bool>,
                              data_par_burst : Option<(usize, usize, usize)>)
                              -> u32 {
     match data_par_burst {
         None                        => {
-            index as u32
+            let meta_enabled = meta_enabled.unwrap_or(true);
+
+            if meta_enabled { index as u32 }
+            else            { SBX_FIRST_DATA_SEQ_NUM + index as u32 }
         },
         Some((data, parity, burst)) => {
+            shadow_to_avoid_use!(meta_enabled);
+
             // the following essentially reverses the index transformation in
             // calc_data_block_write_index
             if burst == 0 {