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

CI: Address clippy lints and upgrade tools so CI succeeds #2149

Merged
merged 7 commits into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ jobs:
~/.cargo/bin/cargo-audit
~/.cargo/.crates.toml
~/.cargo/.crates2.json
key: ${{ runner.os }}-v2-cargo-audit-locked-0.18.3
key: ${{ runner.os }}-v2-cargo-audit-locked-0.20.1

- run: cargo install cargo-audit --locked --vers "0.18.3"
- run: cargo install cargo-audit --locked --vers "0.20.1"

- uses: briansmith/actions-checkout@v4
with:
Expand All @@ -74,9 +74,9 @@ jobs:
~/.cargo/bin/cargo-semver-checks
~/.cargo/.crates.toml
~/.cargo/.crates2.json
key: ${{ runner.os }}-v2-cargo-audit-locked-0.18.3
key: ${{ runner.os }}-v2-cargo-semver-checks-0.35.0

- run: cargo install cargo-semver-checks --locked --vers "0.31.0"
- run: cargo install cargo-semver-checks --locked --vers "0.35.0"

- uses: briansmith/actions-checkout@v4
with:
Expand All @@ -99,9 +99,9 @@ jobs:
~/.cargo/bin/cargo-deny
~/.cargo/.crates.toml
~/.cargo/.crates2.json
key: ${{ runner.os }}-v2-cargo-deny-locked-0.14.20
key: ${{ runner.os }}-v2-cargo-deny-locked-0.16.1

- run: cargo install cargo-deny --locked --vers "0.14.20"
- run: cargo install cargo-deny --locked --vers "0.16.1"

- uses: briansmith/actions-checkout@v4
with:
Expand Down
9 changes: 0 additions & 9 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@ style guidelines for that code are in the second section of this document.
*ring* usually follows the [Rust Guidelines](https://aturon.github.io/), but
there are some differences and *ring* adds additional guidelines.

## Imports (`use`) and Qualification

In general, import modules, not non-module items, e.g. `use core`, not
`use core::mem::size_of_val`. This means that the uses of such functions must
be qualified: `core::mem::size_of_val(x)`, not `size_of_val(x)`. Exceptions may
be made for things that are very annoying to qualify; for example, we usually
`use super::input::*` or `use super::input::Input` because writing things like
`input::Input` is highly annoying.

## Submodules and file naming.

In general, avoid nesting modules and avoid exporting any non-module items from
Expand Down
14 changes: 9 additions & 5 deletions mk/cargo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ qemu_x86_64="qemu-x86_64"
# be needed to compile the build script, or to compile for other targets.
if [ -n "${ANDROID_HOME-}" ]; then
# Keep the next line in sync with the corresponding line in install-build-tools.sh.
ndk_version=25.2.9519653
ndk_version=27.1.12297006
ANDROID_NDK_ROOT=${ANDROID_NDK_ROOT:-${ANDROID_HOME}/ndk/$ndk_version}
fi
if [ -n "${ANDROID_NDK_ROOT-}" ]; then
Expand All @@ -60,14 +60,14 @@ for arg in $*; do
done

# See comments in install-build-tools.sh.
llvm_version=18
llvm_version=19

use_clang=
case $target in
aarch64-linux-android)
export CC_aarch64_linux_android=$android_tools/aarch64-linux-android21-clang
export AR_aarch64_linux_android=$android_tools/llvm-ar
export CARGO_TARGET_AARCH64_LINUX_ANDROID_LINKER=$android_tools/aarch64-linux-android21-clang
export CARGO_TARGET_AARCH64_LINUX_ANDROID_LINKER=$CC_aarch64_linux_android
;;
aarch64-unknown-linux-gnu)
use_clang=1
Expand All @@ -94,9 +94,13 @@ case $target in
export CARGO_TARGET_ARM_UNKNOWN_LINUX_GNUEABIHF_RUNNER="$qemu_arm_gnueabihf"
;;
armv7-linux-androideabi)
export CC_armv7_linux_androideabi=$android_tools/armv7a-linux-androideabi19-clang
# https://github.com/android/ndk/wiki/Changelog-r26#announcements says API
# level 21 is the minimum supported as of NDK 26, even though we'd like to
# support API level 19. Rust 1.82 is doing the same; see
# https://github.com/rust-lang/rust/commit/6ef11b81c2c02c3c4b7556d1991a98572fe9af87.
export CC_armv7_linux_androideabi=$android_tools/armv7a-linux-androideabi21-clang
export AR_armv7_linux_androideabi=$android_tools/llvm-ar
export CARGO_TARGET_ARMV7_LINUX_ANDROIDEABI_LINKER=$android_tools/armv7a-linux-androideabi19-clang
export CARGO_TARGET_ARMV7_LINUX_ANDROIDEABI_LINKER=$CC_armv7_linux_androideabi
;;
armv7-unknown-linux-gnueabihf)
export CC_armv7_unknown_linux_gnueabihf=arm-linux-gnueabihf-gcc
Expand Down
19 changes: 10 additions & 9 deletions mk/install-build-tools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,18 @@ case ${target-} in
# https://blog.rust-lang.org/2023/01/09/android-ndk-update-r25.html says
# "Going forward the Android platform will target the most recent LTS NDK,
# allowing Rust developers to access platform features sooner. These updates
# should occur yearly and will be announced in release notes." Assume that
# means that we should always prefer to be using the latest 25.x.y version of
# the NDK until the Rust project announces that we should use a higher major
# version number.
# should occur yearly and will be announced in release notes."
#
# TODO: This should probably be implemented as a map of Rust toolchain version
# to NDK version; e.g. our MSRV might (only) support an older NDK than the
# latest stable Rust toolchain.
# https://github.com/actions/runner-images/issues/10614 indicates that GitHub
# actions doesn't intend to keep unsupported versions around, so in general
# we'll end up only supporting the latest NDK even for MSRV builds.
#
# https://developer.android.com/ndk/guides/other_build_systems explains how
# to set the API level.
#
# Keep the following line in sync with the corresponding line in cargo.sh.
ndk_version=25.2.9519653
#
ndk_version=27.1.12297006

mkdir -p "${ANDROID_HOME}/licenses"
android_license_file="${ANDROID_HOME}/licenses/android-sdk-license"
Expand Down Expand Up @@ -208,7 +209,7 @@ case "${OSTYPE-}" in
linux*)
if [ -n "$use_clang" ]; then
ubuntu_codename=$(lsb_release --codename --short)
llvm_version=18
llvm_version=19
sudo apt-key add mk/llvm-snapshot.gpg.key
sudo add-apt-repository "deb http://apt.llvm.org/$ubuntu_codename/ llvm-toolchain-$ubuntu_codename-$llvm_version main"
sudo apt-get update
Expand Down
4 changes: 2 additions & 2 deletions src/aead/chacha/fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// Adapted from the BoringSSL crypto/chacha/chacha.c.

use super::{Counter, Key, BLOCK_LEN};
use core::ops::RangeFrom;
use core::{mem::size_of, ops::RangeFrom};

pub(super) fn ChaCha20_ctr32(
key: &Key,
Expand Down Expand Up @@ -82,7 +82,7 @@ fn chacha_core(output: &mut [u8; BLOCK_LEN], input: &State) {
}

output
.chunks_exact_mut(core::mem::size_of::<u32>())
.chunks_exact_mut(size_of::<u32>())
.zip(x.iter())
.for_each(|(output, &x)| output.copy_from_slice(&x.to_le_bytes()));
}
Expand Down
3 changes: 2 additions & 1 deletion src/arithmetic/constant.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::limb::Limb;
use core::mem::size_of;

const fn parse_digit(d: u8) -> u8 {
match d.to_ascii_lowercase() {
Expand All @@ -12,7 +13,7 @@
pub const fn limbs_from_hex<const LIMBS: usize>(hex: &str) -> [Limb; LIMBS] {
let hex = hex.as_bytes();
let mut limbs = [0; LIMBS];
let limb_nibbles = core::mem::size_of::<Limb>() * 2;
let limb_nibbles = size_of::<Limb>() * 2;

Check warning on line 16 in src/arithmetic/constant.rs

View check run for this annotation

Codecov / codecov/patch

src/arithmetic/constant.rs#L16

Added line #L16 was not covered by tests
let mut i = 0;

while i < hex.len() {
Expand Down
9 changes: 3 additions & 6 deletions src/bssl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,13 @@ impl From<Result> for core::result::Result<(), error::Unspecified> {
mod tests {
mod result {
use crate::{bssl, c};
use core::mem;
use core::mem::{align_of, size_of};

#[test]
fn size_and_alignment() {
type Underlying = c::int;
assert_eq!(mem::size_of::<bssl::Result>(), mem::size_of::<Underlying>());
assert_eq!(
mem::align_of::<bssl::Result>(),
mem::align_of::<Underlying>()
);
assert_eq!(size_of::<bssl::Result>(), size_of::<Underlying>());
assert_eq!(align_of::<bssl::Result>(), align_of::<Underlying>());
}

#[test]
Expand Down
3 changes: 2 additions & 1 deletion src/cpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

pub(crate) use self::features::Features;
use core::mem::size_of;

macro_rules! impl_get_feature {
{ $feature:path => $T:ident } => {
Expand Down Expand Up @@ -96,7 +97,7 @@ mod features {
}
}

const _: () = assert!(core::mem::size_of::<Features>() == 0);
const _: () = assert!(size_of::<Features>() == 0);

cfg_if::cfg_if! {
if #[cfg(any(target_arch = "aarch64", target_arch = "arm"))] {
Expand Down
7 changes: 4 additions & 3 deletions src/cpu/arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

mod abi_assumptions {
use core::mem::size_of;

// TODO: Support ARM64_32; see
// https://github.com/briansmith/ring/issues/1832#issuecomment-1892928147. This also requires
// replacing all `cfg(target_pointer_width)` logic for non-pointer/reference things
Expand All @@ -21,9 +23,8 @@ mod abi_assumptions {
const _ASSUMED_POINTER_SIZE: usize = 8;
#[cfg(target_arch = "arm")]
const _ASSUMED_POINTER_SIZE: usize = 4;
const _ASSUMED_USIZE_SIZE: () = assert!(core::mem::size_of::<usize>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_REF_SIZE: () =
assert!(core::mem::size_of::<&'static u8>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_USIZE_SIZE: () = assert!(size_of::<usize>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_REF_SIZE: () = assert!(size_of::<&'static u8>() == _ASSUMED_POINTER_SIZE);

// To support big-endian, we'd need to make several changes as described in
// https://github.com/briansmith/ring/issues/1832.
Expand Down
7 changes: 4 additions & 3 deletions src/cpu/intel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use cfg_if::cfg_if;

mod abi_assumptions {
use core::mem::size_of;

// TOOD: Support targets that do not have SSE and SSE2 enabled, such as
// x86_64-unknown-linux-none. See
// https://github.com/briansmith/ring/issues/1793#issuecomment-1793243725,
Expand All @@ -27,9 +29,8 @@ mod abi_assumptions {
const _ASSUMED_POINTER_SIZE: usize = 8;
#[cfg(target_arch = "x86")]
const _ASSUMED_POINTER_SIZE: usize = 4;
const _ASSUMED_USIZE_SIZE: () = assert!(core::mem::size_of::<usize>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_REF_SIZE: () =
assert!(core::mem::size_of::<&'static u8>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_USIZE_SIZE: () = assert!(size_of::<usize>() == _ASSUMED_POINTER_SIZE);
const _ASSUMED_REF_SIZE: () = assert!(size_of::<&'static u8>() == _ASSUMED_POINTER_SIZE);

const _ASSUMED_ENDIANNESS: () = assert!(cfg!(target_endian = "little"));
}
Expand Down
5 changes: 3 additions & 2 deletions src/digest/dynstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use super::{format_output, sha1, sha2, Output};
use crate::{cpu, polyfill::slice};
use core::mem::size_of;

// Invariant: When constructed with `new32` (resp. `new64`), `As32` (resp.
// `As64`) is the active variant.
Expand Down Expand Up @@ -92,7 +93,7 @@ pub(super) fn sha256_format_output(state: DynState) -> Output {
unreachable!();
}
};
format_output::<_, _, { core::mem::size_of::<u32>() }>(state, u32::to_be_bytes)
format_output::<_, _, { size_of::<u32>() }>(state, u32::to_be_bytes)
}

pub(super) fn sha512_format_output(state: DynState) -> Output {
Expand All @@ -102,5 +103,5 @@ pub(super) fn sha512_format_output(state: DynState) -> Output {
unreachable!();
}
};
format_output::<_, _, { core::mem::size_of::<u64>() }>(state, u64::to_be_bytes)
format_output::<_, _, { size_of::<u64>() }>(state, u64::to_be_bytes)
}
5 changes: 3 additions & 2 deletions src/polyfill/array_flat_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ where
#[cfg(test)]
mod tests {
use super::*;
use core::mem::size_of;

#[test]
fn test_array_flat_map() {
Expand Down Expand Up @@ -109,7 +110,7 @@ mod tests {
}
impl ExactSizeIterator for DownwardCounter {}

const MAX: usize = usize::MAX / core::mem::size_of::<usize>();
const MAX: usize = usize::MAX / size_of::<usize>();

static TEST_CASES: &[(usize, bool)] = &[(MAX, true), (MAX + 1, false)];
TEST_CASES.iter().copied().for_each(|(input_len, is_some)| {
Expand All @@ -119,7 +120,7 @@ mod tests {
let mapped = ArrayFlatMap::new(inner, usize::to_be_bytes);
assert_eq!(mapped.is_some(), is_some);
if let Some(mapped) = mapped {
assert_eq!(mapped.len(), input_len * core::mem::size_of::<usize>());
assert_eq!(mapped.len(), input_len * size_of::<usize>());
}
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/polyfill/cstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

#![cfg(all(target_arch = "aarch64", target_vendor = "apple"))]

use core::mem::{align_of, size_of};

// TODO(MSRV 1.64): Use `core::ffi::c_char`.
use libc::c_char;

Expand All @@ -37,10 +39,8 @@ pub struct Ref(&'static [u8]);
impl Ref {
#[inline(always)]
pub fn as_ptr(&self) -> *const c_char {
const _SAME_ALIGNMENT: () =
assert!(core::mem::align_of::<u8>() == core::mem::align_of::<c_char>());
const _SAME_SIZE: () =
assert!(core::mem::size_of::<u8>() == core::mem::size_of::<c_char>());
const _SAME_ALIGNMENT: () = assert!(align_of::<u8>() == align_of::<c_char>());
const _SAME_SIZE: () = assert!(size_of::<u8>() == size_of::<c_char>());

// It is safe to cast a `*const u8` to a `const c_char` as they are the
// same size and alignment.
Expand Down
4 changes: 2 additions & 2 deletions src/polyfill/notsend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::test;
use core::marker::PhantomData;
use core::{marker::PhantomData, mem::size_of};

/// A ZST that can be added to any type to make the type `!Send`.
#[derive(Clone, Copy)]
Expand All @@ -25,4 +25,4 @@ impl NotSend {

const _: () = test::compile_time_assert_clone::<NotSend>();
const _: () = test::compile_time_assert_copy::<NotSend>();
const _: () = assert!(core::mem::size_of::<NotSend>() == 0);
const _: () = assert!(size_of::<NotSend>() == 0);
6 changes: 4 additions & 2 deletions src/polyfill/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.

use core::mem::size_of;

// TODO(MSRV feature(slice_as_chunks)): Use `slice::as_chunks` instead.
// This is copied from the libcore implementation of `slice::as_chunks`.
#[inline(always)]
Expand Down Expand Up @@ -56,7 +58,7 @@ pub fn as_chunks_mut<T, const N: usize>(slice: &mut [T]) -> (&mut [[T; N]], &mut
// TODO(MSRV feature(slice_flatten)): Use `slice::flatten` instead.
// This is derived from the libcore implementation, using only stable APIs.
pub fn flatten<T, const N: usize>(slice: &[[T; N]]) -> &[T] {
let len = if core::mem::size_of::<T>() == 0 {
let len = if size_of::<T>() == 0 {
slice.len().checked_mul(N).expect("slice len overflow")
} else {
// SAFETY: `slice.len() * N` cannot overflow because `slice` is
Expand All @@ -70,7 +72,7 @@ pub fn flatten<T, const N: usize>(slice: &[[T; N]]) -> &[T] {
// TODO(MSRV feature(slice_flatten)): Use `slice::flatten_mut` instead.
// This is derived from the libcore implementation, using only stable APIs.
pub fn flatten_mut<T, const N: usize>(slice: &mut [[T; N]]) -> &mut [T] {
let len = if core::mem::size_of::<T>() == 0 {
let len = if size_of::<T>() == 0 {
slice.len().checked_mul(N).expect("slice len overflow")
} else {
// SAFETY: `slice.len() * N` cannot overflow because `slice` is
Expand Down
6 changes: 3 additions & 3 deletions src/rsa/keypair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,13 @@ impl KeyPair {
/// performance reasons and to avoid any side channels that such tests
/// would provide.
/// * Section 6.4.1.2.1, Step 6, and 6.4.1.4.3, Step 7:
/// * *ring* has a slightly looser lower bound for the values of `p`
/// * *ring* has a slightly looser lower bound for the values of `p`
/// and `q` than what the NIST document specifies. This looser lower
/// bound matches what most other crypto libraries do. The check might
/// be tightened to meet NIST's requirements in the future. Similarly,
/// the check that `p` and `q` are not too close together is skipped
/// currently, but may be added in the future.
/// - The validity of the mathematical relationship of `dP`, `dQ`, `e`
/// * The validity of the mathematical relationship of `dP`, `dQ`, `e`
/// and `n` is verified only during signing. Some size checks of `d`,
/// `dP` and `dQ` are performed at construction, but some NIST checks
/// are skipped because they would be expensive and/or they would leak
Expand All @@ -207,7 +207,7 @@ impl KeyPair {
/// necessary, that can be done by signing any message with the key
/// pair.
///
/// * `d` is not fully validated, neither at construction nor during
/// * `d` is not fully validated, neither at construction nor during
/// signing. This is OK as far as *ring*'s usage of the key is
/// concerned because *ring* never uses the value of `d` (*ring* always
/// uses `p`, `q`, `dP` and `dQ` via the Chinese Remainder Theorem,
Expand Down
Loading