Introduce functions to read the values in the fst file. - #3
Conversation
| /// 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> { | ||
| let mut buf = vec![0; 1024]; // Allocate a buffer for the value |
There was a problem hiding this comment.
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.
| 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()? }; |
There was a problem hiding this comment.
- The
resultshould be checked, sincefstReaderGetValueFromHandleAtTimemay return a null pointer. - The return type of this method should be
Box<[u8]>instead ofString, to be consistent with the writer API of this Rust crate.
| }; | ||
|
|
||
| let value_str = unsafe { std::ffi::CStr::from_ptr(result as *const raw::c_char).to_str().ok()? }; | ||
| return Some(value_str.to_owned()); |
There was a problem hiding this comment.
Don't use return at the end of the function, just use the expression without the trailing semicolon.
| /// | ||
| /// 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> { |
There was a problem hiding this comment.
The method could have been named something simpler, like value_at_time.
| } | ||
|
|
||
| /// 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> { |
There was a problem hiding this comment.
The name of this method could be find_next_time_change.
| let mut time_table_len = 0; | ||
| let mut last_time = 0; | ||
|
|
||
| self.for_each_block(|time, current_handle, _val, _| { |
There was a problem hiding this comment.
| 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; | ||
| } |
There was a problem hiding this comment.
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:
| 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); | |
| } |
|
Thanks for your contribution! I've added some comments to your changes, please check. |
|
Hi, thanks for your quick response and feeback. My first time doing things in rust... Do you think it would make sense to PR GTKWave for the I will update the small rust changes in the coming days. |
Actually the fstapi in this repo is already a modified version, lol. In order to make the Rust binding work faster and more straightforward, I have modified some APIs and added some new. So feel free to change And IMO it's not make sense to add So the point is that int fstReaderGetValueFromHandleAtTime(void *ctx, uint64_t tim, fstHandle facidx, char *buf, size_t buf_len);Or allocate memory of sufficient length within the API: char *fstReaderGetValueFromHandleAtTime(void *ctx, uint64_t tim, fstHandle facidx); |
|
Ok, I will make the changes here then. |
2 participants
This pull request introduces two functions to read the values of the opened fst file. The first just returns the value at a given time and the second gets the value, as soon as it changed.