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

revert undefined behavior introduction (recommended by clippy?) #1020

Merged

Conversation

danielrh
Copy link
Contributor

Hi folks
In a recent revision of rust-sdl2 my art tool ( https://github.com/danielrh/stamps ) was beginning to get increasingly difficult to track undefined behavior, especially on release.

After trying everything I knew of with the SDL 2.0.10 upgrade--I determined it was likely to be in the rust layer.
So I bisected the code and found the offending diff was a9893f0 which returned the address of a local variable when getting the raw data out of a rect

I've attached a reverting change here which restores my program to normal operations

@antonok-edm
Copy link
Contributor

Wow, sorry about that! For reference (no pun intended), this is the clippy lint in question.

I hadn't looked at it too closely at the time, but it definitely doesn't make sense to get a raw pointer to a temporarily Copyed object that gets immediately dropped.

It looks like that lint has been downgraded to "pedantic" since I made that commit, but the behavior still exists - I've just opened rust-lang/rust-clippy#5953.

@Cobrand
Copy link
Member

Cobrand commented Aug 24, 2020

Nice catch, can you add something like #[allow(trivially_copy_pass_by_ref)] so that clippy does not catch this one in the future? (I have no idea if this is the right syntax for clippy). It's likely in a year or two this will get overlooked and happen again without an "allow" part.

You might want to link this issue next to it as well so that we know what it's for.

@danielrh danielrh force-pushed the remove_formatting_undefined_behavior branch from e6eaae4 to eaa01c5 Compare August 24, 2020 22:28
@danielrh
Copy link
Contributor Author

ok I've updated the diff to suppress the warning in all raw() functions that return pointers.
Hopefully y'all can merge it soon and do another cargo package release--I'd like to get back on the official build

@Cobrand Cobrand merged commit 11cfb8d into Rust-SDL2:master Aug 24, 2020
@Cobrand
Copy link
Member

Cobrand commented Aug 24, 2020

Thanks! 0.34.3 should be live now.

@danielrh
Copy link
Contributor Author

Thank you for your swift action here!! Much appreciated

sypwex pushed a commit to sypwex/rust-sdl2 that referenced this pull request Jun 2, 2024
…efined_behavior

revert undefined behavior introduction (recommended by clippy?)
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.

3 participants