Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions fstapi/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,41 @@ impl Reader {
_ => Err(Error::InvalidOperation),
}
}
/// Gets the value of the variable with the given handle at the specified time.
///
/// If the variable is not found, returns `None`.
/// If the value is not found, returns `None`.
pub fn get_value_from_handle_at_time(&mut self, time: u64, handle: Handle) -> Option<String> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method could have been named something simpler, like value_at_time.

let mut buf = vec![0; 1024]; // Allocate a buffer for the value

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The length of this buffer should be the length of the signal corresponding to the handle, rather than hardcoded, otherwise there is a risk of buffer overflow.

I checked the fstapi code, it seems that the signal lengths are stored in an array in the memory pointed to by the reader context, but there is no such API to read them, and unfortunately the memory layout of the context is opaque.

It would be better to add an API called fstReaderGetSignalLength to fstapi.c and fstapi.h, that returns xc->signal_lens[handle], and uses the the return value as the length to initialize the buffer.

let result = unsafe {
capi::fstReaderGetValueFromHandleAtTime(self.ctx, time, handle.into(), buf.as_mut_ptr())
};

let value_str = unsafe { std::ffi::CStr::from_ptr(result as *const raw::c_char).to_str().ok()? };

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The result should be checked, since fstReaderGetValueFromHandleAtTime may return a null pointer.
  • The return type of this method should be Box<[u8]> instead of String, to be consistent with the writer API of this Rust crate.

return Some(value_str.to_owned());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use return at the end of the function, just use the expression without the trailing semicolon.

}

/// Finds the next time at which a variable changes after a given time.
pub fn get_next_time_change(&mut self, start_time: u64, handle: Handle) -> Result<u64> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of this method could be find_next_time_change.

let mut time_table = Vec::new();
let mut time_table_len = 0;
let mut last_time = 0;

self.for_each_block(|time, current_handle, _val, _| {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.for_each_block(|time, current_handle, _val, _| {
self.for_each_block(|time, current_handle, _, _| {

if time >= start_time && current_handle == handle {
time_table.push(time);
time_table_len += 1;
}
Comment on lines +263 to +266

@MaxXSoft MaxXSoft Nov 19, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both time_table and time_table_len can be removed, since you only want to get the first time that meets the condition.

Try to define a variable let mut first_time = None; and rewrite with:

Suggested change
if time >= start_time && current_handle == handle {
time_table.push(time);
time_table_len += 1;
}
if first_time.is_none() && time >= start_time && current_handle == handle {
first_time = Some(time);
}

last_time = time;
})?;

let next_time = if time_table_len > 0 {
time_table[0]
} else {
last_time
};
Ok(next_time)
}
}

impl Drop for Reader {
Expand Down