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

Use explicit u8 when assigning a byte slice #334

Merged
merged 1 commit into from Nov 2, 2021

Conversation

junderw
Copy link
Contributor

@junderw junderw commented Oct 17, 2021

Is there a way to tell the compiler to not allow [0; 64] and require that either the type is explicitly given to the variable, or that each member uses explicit 0u8 notation?

I noticed the usage was a mix of explicit and implicit, so I changed all to explicit.

@real-or-random
Copy link
Collaborator

Is there a way to tell the compiler to not allow [0; 64] and require that either the type is explicitly given to the variable, or that each member uses explicit 0u8 notation?

It's maybe useful as a formatting convention but it shouldn't add too much:

If an integer type can be uniquely determined from the surrounding program context, the unsuffixed integer literal has that type.

(https://doc.rust-lang.org/stable/reference/tokens.html#number-literals)

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 24d6f62

Agreed that it's clearer this way and I wish there was a way to lint for it.

@apoelstra apoelstra merged commit 6a774bd into rust-bitcoin:master Nov 2, 2021
@thomaseizinger
Copy link
Contributor

I noticed the usage was a mix of explicit and implicit, so I changed all to explicit.

Was there a rationale for not dealing with the following occurrences?

❯ git grep -e '\[0;' master
master:secp256k1-sys/src/lib.rs:        Self::from_array_unchecked([0; 64])
master:secp256k1-sys/src/lib.rs:        Self::from_array_unchecked([0; 64])
master:secp256k1-sys/src/lib.rs:        Self::from_array_unchecked([0; 64])
master:secp256k1-sys/src/lib.rs:        Self::from_array_unchecked([0; 96])
master:secp256k1-sys/src/lib.rs:    static mut PREALLOCATED_CONTEXT: [u8; CTX_SIZE] = [0; CTX_SIZE];
master:secp256k1-sys/src/recovery.rs:    pub fn new() -> RecoverableSignature { RecoverableSignature([0; 65]) }
master:src/key.rs:            let mut result = vec![0; $hex.len() / 2];
master:src/key.rs:        assert_eq!(SecretKey::from_slice(&[0; 32]), Err(InvalidSecretKey));
master:src/key.rs:            PublicKey::from_slice(&[0; constants::PUBLIC_KEY_SIZE - 1]),
master:src/key.rs:            PublicKey::from_slice(&[0; constants::PUBLIC_KEY_SIZE + 1]),
master:src/key.rs:            PublicKey::from_slice(&[0; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE - 1]),
master:src/key.rs:            PublicKey::from_slice(&[0; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE + 1]),
master:src/key.rs:            SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE - 1]),
master:src/key.rs:            SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE + 1]),
master:src/lib.rs:            let mut result = vec![0; $hex.len() / 2];
master:src/lib.rs:        assert_eq!(Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE + 1]),
master:src/lib.rs:        assert_eq!(Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE]),
master:src/lib.rs:        assert_eq!(Message::from_slice(&[0; constants::MESSAGE_SIZE - 1]),
master:src/lib.rs:        assert_eq!(Message::from_slice(&[0; constants::MESSAGE_SIZE + 1]),
master:src/lib.rs:        assert!(Message::from_slice(&[0; constants::MESSAGE_SIZE]).is_ok());
master:src/recovery.rs:        let sig = RecoverableSignature::from_compact(&[0; 64], RecoveryId(0)).unwrap();
master:src/schnorrsig.rs:            PublicKey::from_slice(&[0; constants::SCHNORRSIG_PUBLIC_KEY_SIZE - 1]),
master:src/schnorrsig.rs:            PublicKey::from_slice(&[0; constants::SCHNORRSIG_PUBLIC_KEY_SIZE + 1]),

Asking because I am currently rebasing #327 and when trying to re-apply my changes on top of this, I noticed that a simple search and replace hit more lines than I expected.

@junderw
Copy link
Contributor Author

junderw commented Nov 11, 2021

I probably should have documented the command I used to get what I did.

I definitely intended to replace all.

If you don't want to make another PR I can.

Thanks for the catch.

@apoelstra
Copy link
Member

I guess we could write a shell script that runs this grep command in CI.

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.

None yet

4 participants