Skip to content

Commit

Permalink
Fixes to reviews
Browse files Browse the repository at this point in the history 
* Support iterator
* Change read_at as method
* sub_size returns Result
* Check alignment on get_item_count
* Snake names
* Some typos
  • Loading branch information
@XuJiandong
XuJiandong committed Aug 23, 2023
1 parent 8e11ef1 commit 703b9e9
Show file tree
Hide file tree
Showing 10 changed files with 213 additions and 120 deletions.
1 change: 0 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ clippy:
@set -eu; \
for dir in ${RUST_PROJS}; do \
cd "$${dir}"; \
cargo clean; \
cargo clippy --all --all-targets --all-features; \
cd - > /dev/null; \
done
Expand Down
134 changes: 74 additions & 60 deletions bindings/rust/src/lazy_reader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,49 +58,6 @@ pub struct Union {
pub cursor: Cursor,
}

pub fn read_at(cur: &Cursor, buf: &mut [u8]) -> Result<usize, Error> {
let read_len = min(cur.size, buf.len());
let ds = &mut *cur.data_source.borrow_mut();
if read_len > ds.cache.len() {
return ds.reader.read(buf, cur.offset);
}
if cur.offset < ds.cache_start_point
|| (cur.offset + read_len) > (ds.cache_start_point + ds.cache_size)
{
let reader = &ds.reader;
let size = reader.read(&mut ds.cache[..], cur.offset).unwrap();
if size < read_len {
return Err(Error::Read(format!(
"read_at `if size({}) < read_len({})`",
size, read_len
)));
}
ds.cache_size = size;
ds.cache_start_point = cur.offset;

if ds.cache_size > ds.cache.len() {
return Err(Error::Read(format!(
"read_at `if ds.cache_size({}) > ds.cache.len()({})`",
ds.cache_size,
ds.cache.len()
)));
}
}
if cur.offset < ds.cache_start_point || (cur.offset - ds.cache_start_point) > ds.cache.len() {
return Err(Error::Read(
"read_at `if cur.offset < ds.start_point || ...`".into(),
));
}
let read_point = cur.offset - ds.cache_start_point;
if read_point + read_len > ds.cache_size {
return Err(Error::Read(
"read_at `if read_point + read_len > ds.cache_size`".into(),
));
}
buf.copy_from_slice(&ds.cache[read_point..(read_point + read_len)]);
Ok(read_len)
}

impl Cursor {
/**
cache_size: normally it can be set to MAX_CACHE_SIZE(2K)
Expand All @@ -122,13 +79,63 @@ impl Cursor {
data_source: Rc::new(RefCell::new(data_source)),
}
}
pub fn read_at(&self, buf: &mut [u8]) -> Result<usize, Error> {
let read_len = min(self.size, buf.len());
let ds = &mut *self.data_source.borrow_mut();
if read_len > ds.cache.len() {
return ds.reader.read(buf, self.offset);
}
if self.offset < ds.cache_start_point
|| (self.offset + read_len) > (ds.cache_start_point + ds.cache_size)
{
let reader = &ds.reader;
let size = reader.read(&mut ds.cache[..], self.offset).unwrap();
if size < read_len {
return Err(Error::Read(format!(
"read_at `if size({}) < read_len({})`",
size, read_len
)));
}
ds.cache_size = size;
ds.cache_start_point = self.offset;

if ds.cache_size > ds.cache.len() {
return Err(Error::Read(format!(
"read_at `if ds.cache_size({}) > ds.cache.len()({})`",
ds.cache_size,
ds.cache.len()
)));
}
}
if self.offset < ds.cache_start_point
|| (self.offset - ds.cache_start_point) > ds.cache.len()
{
return Err(Error::Read(
"read_at `if cur.offset < ds.start_point || ...`".into(),
));
}
let read_point = self.offset - ds.cache_start_point;
if read_point + read_len > ds.cache_size {
return Err(Error::Read(
"read_at `if read_point + read_len > ds.cache_size`".into(),
));
}
buf.copy_from_slice(&ds.cache[read_point..(read_point + read_len)]);
Ok(read_len)
}

pub fn add_offset(&mut self, offset: usize) {
self.offset = self.offset.checked_add(offset).unwrap();
}

pub fn sub_size(&mut self, shrink_size: usize) {
self.size = self.size.checked_sub(shrink_size).unwrap();
pub fn sub_size(&mut self, shrink_size: usize) -> Result<(), Error> {
self.size = self.size.checked_sub(shrink_size).ok_or_else(|| {
Error::Overflow(format!(
"sub_size: self.size({}) - shrink_size({}) failed",
self.size, shrink_size
))
})?;
Ok(())
}

pub fn validate(&self) -> Result<(), Error> {
Expand All @@ -147,7 +154,7 @@ impl Cursor {

pub fn unpack_number(&self) -> Result<usize, Error> {
let mut src = [0u8; 4];
let size = read_at(self, &mut src[..]).unwrap();
let size = self.read_at(&mut src[..])?;
if size != 4 {
Err(Error::FieldCount("unpack_number".into()))
} else {
Expand Down Expand Up @@ -210,14 +217,21 @@ impl Cursor {
} else {
let mut cur2 = self.clone();
cur2.add_offset(NUM_T_SIZE);
cur2.sub_size(NUM_T_SIZE);
cur2.sub_size(NUM_T_SIZE)?;
cur2.validate()?;
cur2.get_item_count()
}
}

pub fn get_item_count(&self) -> Result<usize, Error> {
let count = self.unpack_number()? / 4;
let len = self.unpack_number()?;
if len % 4 != 0 {
return Err(Error::UnknownItem(format!(
"get_item_count not aligned, len = {}",
len
)));
}
let count = len / 4;
if count == 0 {
Err(Error::UnknownItem("get_item_count".into()))
} else {
Expand Down Expand Up @@ -283,7 +297,7 @@ impl Cursor {
res.offset = self.offset;
res.add_offset(item_start);
res.size = total_size;
res.sub_size(item_start)
res.sub_size(item_start)?;
} else {
temp.offset = self.offset;
let calc_offset = calculate_offset(NUM_T_SIZE, item_index + 2, 0);
Expand All @@ -293,7 +307,7 @@ impl Cursor {
res.offset = self.offset;
res.add_offset(item_start);
res.size = item_end;
res.sub_size(item_start);
res.sub_size(item_start)?;
}
res.validate()?;
Ok(res)
Expand Down Expand Up @@ -323,7 +337,7 @@ impl Cursor {
let item_id = self.unpack_number()?;
let mut cursor = self.clone();
cursor.add_offset(NUM_T_SIZE);
cursor.sub_size(NUM_T_SIZE);
cursor.sub_size(NUM_T_SIZE)?;
cursor.validate()?;
Ok(Union { item_id, cursor })
}
Expand All @@ -338,7 +352,7 @@ impl TryFrom<Cursor> for u64 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 8];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_u64".into()))
} else {
Expand All @@ -351,7 +365,7 @@ impl TryFrom<Cursor> for i64 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 8];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_i64".into()))
} else {
Expand All @@ -364,7 +378,7 @@ impl TryFrom<Cursor> for u32 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 4];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_u32".into()))
} else {
Expand All @@ -377,7 +391,7 @@ impl TryFrom<Cursor> for i32 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 4];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_i32".into()))
} else {
Expand All @@ -390,7 +404,7 @@ impl TryFrom<Cursor> for u16 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 2];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_u16".into()))
} else {
Expand All @@ -403,7 +417,7 @@ impl TryFrom<Cursor> for i16 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 2];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_i16".into()))
} else {
Expand All @@ -416,7 +430,7 @@ impl TryFrom<Cursor> for u8 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 1];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_u8".into()))
} else {
Expand All @@ -429,7 +443,7 @@ impl TryFrom<Cursor> for i8 {
type Error = Error;
fn try_from(cur: Cursor) -> Result<Self, Error> {
let mut buf = [0u8; 1];
let size = read_at(&cur, &mut buf[..])?;
let size = cur.read_at(&mut buf[..])?;
if size != buf.len() {
Err(Error::FieldCount("convert_to_i8".into()))
} else {
Expand All @@ -444,7 +458,7 @@ impl TryFrom<Cursor> for Vec<u8> {
let mut buf = Vec::<u8>::new();
buf.resize(cur.size, 0);

let size = read_at(&cur, buf.as_mut_slice())?;
let size = cur.read_at(buf.as_mut_slice())?;
if size != buf.len() {
return Err(Error::Read(format!(
"size({}) != buf.len()({})",
Expand Down
16 changes: 15 additions & 1 deletion examples/lazy-reader-tests/build.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use codegen::{Compiler, Language};
use std::{env, fs, path};
use std::ffi::OsStr;
use std::process::Command;
use std::{
env, fs,
path::{self, PathBuf},
};

fn compile_schema_rust(schema: &str) {
let mut compiler = Compiler::new();
Expand All @@ -10,6 +15,7 @@ fn compile_schema_rust(schema: &str) {
.output_dir(out_dir)
.run()
.unwrap();

println!("cargo:rerun-if-changed={}", schema);
}

Expand All @@ -18,12 +24,20 @@ fn compile_schema_rust_lazy_reader(schema: &str) {
let mut out_dir = path::PathBuf::from(&env::var("OUT_DIR").unwrap_or_else(|_| ".".to_string()));
out_dir.push("lazy_reader");
drop(fs::create_dir(&out_dir));
let mut file_path = out_dir.clone();
compiler
.input_schema_file(schema)
.generate_code(Language::RustLazyReader)
.output_dir(out_dir)
.run()
.unwrap();
file_path.push("types.rs");
Command::new("rustfmt")
.arg(<PathBuf as AsRef<OsStr>>::as_ref(&file_path))
.spawn()
.unwrap()
.wait()
.unwrap();
println!("cargo:rerun-if-changed={}", schema);
}

Expand Down
10 changes: 5 additions & 5 deletions examples/lazy-reader-tests/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl TypesCheckErr {
Self::Mol2Err(v) => v,
}
}
pub fn check_lenght(l1: usize, l2: usize) -> Result<(), Self> {
pub fn check_length(l1: usize, l2: usize) -> Result<(), Self> {
if l1 == l2 {
Ok(())
} else {
Expand All @@ -98,7 +98,7 @@ impl TypesCheckErr {
}

pub fn check_data<T1: Eq + Debug, T: Mol2Vec<RetType = T1>>(d1: &T, d2: &[T1]) -> ResCheckErr {
TypesCheckErr::check_lenght(d1.mol_len()?, d2.len())?;
TypesCheckErr::check_length(d1.mol_len()?, d2.len())?;

for i in 0..d1.mol_len()? {
TypesCheckErr::check_1_data(&d1.mol_get(i)?, &d2[i])?;
Expand All @@ -111,7 +111,7 @@ impl TypesCheckErr {
d1: &T1,
d2: &T2,
) -> ResCheckErr {
TypesCheckErr::check_lenght(d1.mol_len()?, d2.mol_len()?)?;
TypesCheckErr::check_length(d1.mol_len()?, d2.mol_len()?)?;
for i in 0..d1.mol_len()? {
TypesCheckErr::check_1_data(&d1.mol_get(i)?, &d2.mol_get(i)?)?;
}
Expand Down Expand Up @@ -204,12 +204,12 @@ impl TypesUnionA {
match self {
Self::Byte(v) => v.check(&d.as_byte()?),
Self::Word(v) => v.check2(&d.as_word()?.into()),
Self::StructA(v) => v.check(&d.as_structa()?),
Self::StructA(v) => v.check(&d.as_struct_a()?),
Self::Bytes(v) => v.check(&d.as_bytes()?.try_into().unwrap()),
Self::Words(v) => v.check(&d.as_words()?.into()),
Self::Table0(v) => v.check(&d.as_table0()?),
Self::Table6(v) => v.check(&d.as_table6()?),
Self::Table6Opt(v) => v.check(&d.as_table6opt()?),
Self::Table6Opt(v) => v.check(&d.as_table6_opt()?),
}
}
}
1 change: 1 addition & 0 deletions examples/lazy-reader-tests/src/types_api2.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(unused_imports)]
#![allow(dead_code)]

include!(concat!(
env!("OUT_DIR"),
Expand Down
Loading

0 comments on commit 703b9e9

Please sign in to comment.