Skip to content

Conversation

@HEnquist
Copy link
Contributor

This PR fixes the issues clippy finds. The most important ones are the set_property and get_property functions in the audio_unit module. They trigger the not_unsafe_ptr_arg_deref lint because they accept a pointer to an AudioUnit instance, but they cannot check that the pointer is valid, and therefore the caller must be responsible for this. They should be marked as unsafe to reflect this. Instead using the associated methods on the AudioUnit structure is safe and should be the preferred way.

The clippy docs strongly recommend against disabling the not_unsafe_ptr_arg_deref rule, so I marked the functions as unsafe. This is a small breaking change, but this still seems like a better alternative than disabling the rule (and we could also discuss if these functions should be pub at all, but now they are, so removing them from the public api could cause trouble for some users).

@simlay
Copy link
Member

simlay commented Jul 22, 2025

The most important ones are the set_property and get_property functions in the audio_unit module. They trigger the not_unsafe_ptr_arg_deref lint because they accept a pointer to an AudioUnit instance.

Well this is quite important. I'm definitely kicking myself for not having clippy in CI prior to #128 or before releasing 0.13.0.

@HEnquist
Copy link
Contributor Author

I can add a clippy job to ci. And why not rustfmt as well when we are at it. Should I just include that in this pr?

@simlay
Copy link
Member

simlay commented Jul 22, 2025

I can add a clippy job to ci. And why not rustfmt as well when we are at it. Should I just include that in this pr?

Sounds good to me!

@HEnquist
Copy link
Contributor Author

Done :)

Copy link
Member

@simlay simlay left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much!

@simlay simlay merged commit 68bb7d0 into RustAudio:master Jul 25, 2025
7 checks passed
@HEnquist
Copy link
Contributor Author

Sweet, thanks for handling this so quickly :)

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