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

Possible Incompleteness #293

Open
YichiZhang0613 opened this issue Mar 2, 2024 · 2 comments
Open

Possible Incompleteness #293

YichiZhang0613 opened this issue Mar 2, 2024 · 2 comments

Comments

@YichiZhang0613
Copy link

In num-bigint/src/bigint.rs and num-bigint/src/biguint.rs, I think the code should check modulus != 0 and exponent >= 0/modulus != 0 as comments mentioned before use them to avoid possible panic.
num-bigint/src/bigint.rs

/// Panics if the exponent is negative or the modulus is zero.
    pub fn modpow(&self, exponent: &Self, modulus: &Self) -> Self {
        power::modpow(self, exponent, modulus)
    }

num-bigint/src/biguint.rs

/// Panics if the modulus is zero.
    pub fn modpow(&self, exponent: &Self, modulus: &Self) -> Self {
        power::modpow(self, exponent, modulus)
    }
@cuviper
Copy link
Member

cuviper commented Mar 2, 2024

Those nested functions do check their conditions right away:

pub(super) fn modpow(x: &BigInt, exponent: &BigInt, modulus: &BigInt) -> BigInt {
assert!(
!exponent.is_negative(),
"negative exponentiation is not supported!"
);
assert!(
!modulus.is_zero(),
"attempt to calculate with zero modulus!"
);

pub(super) fn modpow(x: &BigUint, exponent: &BigUint, modulus: &BigUint) -> BigUint {
assert!(
!modulus.is_zero(),
"attempt to calculate with zero modulus!"
);

to avoid possible panic.

How would we avoid the panic? These functions have no other way to communicate such an error. We could add checked_modpow variants that return an Option though.

@cuviper
Copy link
Member

cuviper commented Mar 2, 2024

I see that you have opened a few similar issues on different projects. In general, it is not considered a problem in Rust if a function panics when documented preconditions are not met -- it means that it is always considered a bug on the caller.

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

No branches or pull requests

2 participants