From 7e53c36d6b0109685aeda7b55d1d289cb7fec21e Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Wed, 12 May 2021 18:39:59 +0800 Subject: [PATCH] [cache (rust)] Fix bugs in array iteration and text outputs - Fix array indexing - Fix panic on empty array - Remove trailing null characters from the policy name - Change XML tag naming for backward compatibility --- src/cache/check.rs | 14 +++++--------- src/cache/dump.rs | 35 ++++++++++++++++------------------- src/cache/superblock.rs | 2 +- src/cache/xml.rs | 2 +- src/pdata/array_walker.rs | 5 ++++- src/pdata/bitset.rs | 3 +-- 6 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/cache/check.rs b/src/cache/check.rs index 26cda61..5428735 100644 --- a/src/cache/check.rs +++ b/src/cache/check.rs @@ -90,9 +90,7 @@ mod format1 { fn visit(&self, _index: u64, b: ArrayBlock) -> array::Result<()> { let mut errs: Vec = Vec::new(); - for i in 0..b.header.nr_entries as usize { - let m = b.values[i]; - + for m in b.values.iter() { if let Err(e) = self.check_flags(&m) { errs.push(e); } @@ -180,12 +178,10 @@ mod format2 { let mut inner = self.inner.lock().unwrap(); let mut errs: Vec = Vec::new(); - let begin = index as usize * b.header.max_entries as usize; - for i in 0..b.header.nr_entries { - let m = b.values[i as usize]; - - if let Err(e) = self.check_flags(&m, inner.dirty_bits.contains(begin + i as usize)) - { + let cbegin = index as u32 * b.header.max_entries; + let cend = cbegin + b.header.nr_entries; + for (m, cblock) in b.values.iter().zip(cbegin..cend) { + if let Err(e) = self.check_flags(&m, inner.dirty_bits.contains(cblock as usize)) { errs.push(e); } if let Err(e) = self.check_oblock(&m, &mut inner.seen_oblocks) { diff --git a/src/cache/dump.rs b/src/cache/dump.rs index 7a88ceb..22f16fd 100644 --- a/src/cache/dump.rs +++ b/src/cache/dump.rs @@ -47,20 +47,21 @@ mod format1 { impl<'a> ArrayVisitor for MappingEmitter<'a> { fn visit(&self, index: u64, b: ArrayBlock) -> array::Result<()> { - for i in 0..b.header.nr_entries as usize { - let map = b.values[i]; + let cbegin = index as u32 * b.header.max_entries; + let cend = cbegin + b.header.nr_entries; + for (map, cblock) in b.values.iter().zip(cbegin..cend) { if !map.is_valid() { continue; } let m = xml::Map { - cblock: index as u32, + cblock, oblock: map.oblock, dirty: map.is_dirty(), }; let mut inner = self.inner.lock().unwrap(); - inner.valid_mappings.set(index as usize, true); + inner.valid_mappings.set(cblock as usize, true); inner .visitor .mapping(&m) @@ -100,9 +101,7 @@ mod format2 { impl ArrayVisitor for DirtyVisitor { fn visit(&self, index: u64, b: ArrayBlock) -> array::Result<()> { let mut pos = (index as usize * (b.header.max_entries as usize)) << 6; - for i in 0..b.header.nr_entries as usize { - let bits = b.values[i]; - + for bits in b.values.iter() { for bi in 0..64u64 { if pos >= self.nr_entries { break; @@ -152,22 +151,22 @@ mod format2 { impl<'a> ArrayVisitor for MappingEmitter<'a> { fn visit(&self, index: u64, b: ArrayBlock) -> array::Result<()> { - for i in 0..b.header.nr_entries as usize { - let map = b.values[i]; - + let cbegin = index as u32 * b.header.max_entries; + let cend = cbegin + b.header.nr_entries; + for (map, cblock) in b.values.iter().zip(cbegin..cend) { if !map.is_valid() { continue; } let mut inner = self.inner.lock().unwrap(); - let dirty = inner.dirty_bits.contains(index as usize); + let dirty = inner.dirty_bits.contains(cblock as usize); let m = xml::Map { - cblock: index as u32, + cblock, oblock: map.oblock, dirty, }; - inner.valid_mappings.set(index as usize, true); + inner.valid_mappings.set(cblock as usize, true); inner .visitor .mapping(&m) @@ -196,13 +195,13 @@ impl<'a> HintEmitter<'a> { impl<'a> ArrayVisitor for HintEmitter<'a> { fn visit(&self, index: u64, b: ArrayBlock) -> array::Result<()> { - let mut cblock = index as u32 * b.header.max_entries; - for i in 0..b.header.nr_entries as usize { + let cbegin = index as u32 * b.header.max_entries; + let cend = cbegin + b.header.nr_entries; + for (hint, cblock) in b.values.iter().zip(cbegin..cend) { if !self.valid_mappings.contains(cblock as usize) { continue; } - let hint = b.values[i]; let h = xml::Hint { cblock, data: hint.hint.to_vec(), @@ -213,8 +212,6 @@ impl<'a> ArrayVisitor for HintEmitter<'a> { .unwrap() .hint(&h) .map_err(|e| array::value_err(format!("{}", e)))?; - - cblock += 1; } Ok(()) @@ -254,7 +251,7 @@ fn dump_metadata(ctx: &Context, sb: &Superblock, _repair: bool) -> anyhow::Resul uuid: "".to_string(), block_size: sb.data_block_size, nr_cache_blocks: sb.cache_blocks, - policy: std::str::from_utf8(&sb.policy_name[..])?.to_string(), + policy: std::str::from_utf8(&sb.policy_name)?.to_string(), hint_width: sb.policy_hint_size, }; out.superblock_b(&xml_sb)?; diff --git a/src/cache/superblock.rs b/src/cache/superblock.rs index c944439..227fed3 100644 --- a/src/cache/superblock.rs +++ b/src/cache/superblock.rs @@ -104,7 +104,7 @@ fn unpack(data: &[u8]) -> IResult<&[u8], Superblock> { }, block, version, - policy_name: policy_name.to_vec(), + policy_name: policy_name.splitn(2, |c| *c == 0).next().unwrap().to_vec(), policy_version: vec![vsn_major, vsn_minor, vsn_patch], policy_hint_size, metadata_sm_root: metadata_sm_root.to_vec(), diff --git a/src/cache/xml.rs b/src/cache/xml.rs index 8b38f03..cf5fa86 100644 --- a/src/cache/xml.rs +++ b/src/cache/xml.rs @@ -119,7 +119,7 @@ impl MetadataVisitor for XmlWriter { } fn mapping(&mut self, m: &Map) -> Result { - let tag = b"map"; + let tag = b"mapping"; let mut elem = BytesStart::owned(tag.to_vec(), tag.len()); elem.push_attribute(mk_attr(b"cache_block", m.cblock)); elem.push_attribute(mk_attr(b"origin_block", m.oblock)); diff --git a/src/pdata/array_walker.rs b/src/pdata/array_walker.rs index de5f43d..aa314e9 100644 --- a/src/pdata/array_walker.rs +++ b/src/pdata/array_walker.rs @@ -53,7 +53,9 @@ impl<'a, V: Unpack + Copy> NodeVisitor for BlockValueVisitor<'a, V> { keys: &[u64], values: &[u64], ) -> btree::Result<()> { - let mut path = path.to_vec(); + if keys.is_empty() { + return Ok(()); + } // The ordering of array indices had been verified in unpack_node(), // thus checking the upper bound implies key continuity among siblings. @@ -86,6 +88,7 @@ impl<'a, V: Unpack + Copy> NodeVisitor for BlockValueVisitor<'a, V> { array_errs.push(array::io_err(&path, values[i]).index_context(keys[i])); } Ok(b) => { + let mut path = path.to_vec(); path.push(b.loc); match unpack_array_block::(&path, b.get_data()) { Ok(array_block) => { diff --git a/src/pdata/bitset.rs b/src/pdata/bitset.rs index 2586e74..fcd0942 100644 --- a/src/pdata/bitset.rs +++ b/src/pdata/bitset.rs @@ -58,10 +58,9 @@ impl ArrayVisitor for BitsetVisitor { ))); } - for i in 0..b.header.nr_entries as usize { + for bits in b.values.iter() { let end: usize = std::cmp::min(begin + 64, self.nr_bits as usize); let mut mask = 1; - let bits = b.values[i]; for bi in begin..end { self.bits.lock().unwrap().set(bi, bits & mask != 0);