Skip to content

Conversation

@chemicstry
Copy link
Contributor

This adds an immutable version of buffer_mut_as_type() like there is buffer() and buffer_mut().

Also fixes mutability of methods that shouldn't require &mut self. If there was a reason why these methods needed to be mut, let me know.

Copy link
Contributor

@abrown abrown left a comment

Choose a reason for hiding this comment

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

I think this makes sense. I added some notes below for later reference; no need for changes beyond rebasing to make sure CI now passes. Thanks!

///
/// Panics if the returned OpenVINO size will not fit in `usize`.
pub fn byte_len(&mut self) -> Result<usize> {
pub fn byte_len(&self) -> Result<usize> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe these &mut self to &self changes make sense in a Rust context but I propagated the mut due to ie_blob_byte_size's signature (e.g.), which takes a *mut ie_blob_t. I wasn't sure if this was just some bindgen over-reaction or if indeed there was some possible underlying mutation. Looking at ie_blob.h now, I see that both size and byteSize are declared const member functions so this should be safe.


/// Retrieve the [`Blob`]'s data as an immutable slice of bytes.
pub fn buffer(&mut self) -> Result<&[u8]> {
pub fn buffer(&self) -> Result<&[u8]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The original &mut self here was probably a typo.

// read too many bytes, so we manually calculate the resulting slice `size`.
let size = self.byte_len()? / std::mem::size_of::<T>();
let slice = std::slice::from_raw_parts(buffer.__bindgen_anon_1.buffer.cast::<T>(), size);
Ok(slice)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be ideal to de-duplicate this but the from_raw_parts is different so don't worry about it.

@abrown abrown merged commit e403601 into intel:main Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants