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 test harness for RustCrypto rsa crate #4

Closed
wants to merge 1 commit into from
Closed

Add test harness for RustCrypto rsa crate #4

wants to merge 1 commit into from

Conversation

ueno
Copy link
Collaborator

@ueno ueno commented Nov 22, 2023

No description provided.

Signed-off-by: Daiki Ueno <dueno@redhat.com>
@ueno ueno closed this by deleting the head repository Nov 22, 2023
@tomato42
Copy link
Owner

@ueno why delete?

@tomato42
Copy link
Owner

merged as 752907f, thanks!

@haraldh
Copy link

haraldh commented Nov 29, 2023

@ueno @tomato42 I somehow miss std::hint::black_box here for:

        let _ = privkey.decrypt(Pkcs1v15Encrypt, &buffer);

@tomato42
Copy link
Owner

@haraldh Sorry, I'm not a rust developer, so could you explain it like I'm 5?

@haraldh
Copy link

haraldh commented Nov 29, 2023

The compiler knows, that privkey was not modified in that function call and neither buffer.. and since you throw away the return value, in theory the compiler could just optimize away the whole line. This can be prevented by wrapping it with std::hint::black_box()

@tomato42
Copy link
Owner

@haraldh wouldn't that require the compiler to know that the decrypt() has no side-effects? I'd be surprised if that was the default for library APIs...

@haraldh
Copy link

haraldh commented Nov 29, 2023

Well, do you know, if the function has no inline=always? If the rust compiler does not do incremental compilation it might have knowledge of no side effects. Rust libs are not like C libs. Better use the black box hint every time for such cases. It does not hurt and saves you from surprises. These hints exist for other languages also.

@tomato42
Copy link
Owner

@haraldh is this sufficient?: a26abdb

@haraldh
Copy link

haraldh commented Nov 30, 2023

@haraldh is this sufficient?: a26abdb

perfect!

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