Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add get_ref() and get_mut() #42

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Add get_ref() and get_mut() #42

merged 1 commit into from
Jun 24, 2024

Conversation

zinid
Copy link

@zinid zinid commented Jun 20, 2024

Under some circumstances get_mut() is required (e.g. to add more data to the underlying buffer).

Same with get_ref().

@est31
Copy link
Member

est31 commented Jun 22, 2024

There is ways around, like using Rc in combination with *Cell or friends but I suppose this is simpler for you. Could you add a note that all bets are off when you are using these functions, like it is the responsibility of the caller to ensure the data is consistent?

@zinid
Copy link
Author

zinid commented Jun 24, 2024

@est31 I do exactly this now: using Rc + RefCell. However this is not enough and I have to implement the Seek + Read traits, because they are not "proxied" through RefCell and Rc, which makes the code even more convoluted.

But I digress 😄

I added a safety comment to get_mut() as you requested 😄 As for get_ref(), I think adding a note about the caller's responsibility to ensure data consistency is generally unnecessary because get_ref() only returns an immutable reference to the underlying reader.

@est31 est31 merged commit 4b22c17 into RustAudio:master Jun 24, 2024
5 of 9 checks passed
@est31
Copy link
Member

est31 commented Jun 24, 2024

get_ref() only returns an immutable reference to the underlying reader.

If you have Cell around then an immutable reference is enough. Same goes for file descriptors. But whatever, I've merged this.

@zinid zinid deleted the add-get-mut branch June 24, 2024 13:40
@zinid
Copy link
Author

zinid commented Jun 24, 2024

Thanks a lot!

Any chance we get this updated in crates.io? 😄

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