Skip to content

Commit

Permalink
Address various warnings new in clang-tidy 19
Browse files Browse the repository at this point in the history
  • Loading branch information
randombit committed Jan 14, 2025
1 parent 985653f commit f24d5af
Show file tree
Hide file tree
Showing 18 changed files with 84 additions and 48 deletions.
11 changes: 10 additions & 1 deletion src/.clang-tidy
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
# This file was automatically generated by src/scripts/dev_tools/run_clang_tidy.py --regenerate-inline-config-file
# This file was automatically generated by ./src/scripts/dev_tools/run_clang_tidy.py --regenerate-inline-config-file
#
# All manual edits to this file will be lost. Edit the script
# then regenerate this configuration file.
Expand All @@ -19,6 +19,7 @@ Checks: >
-*-member-init,
-bugprone-lambda-function-name,
-bugprone-unchecked-optional-access,
-bugprone-empty-catch,
-cert-err58-cpp,
-cppcoreguidelines-avoid-const-or-ref-data-members,
-cppcoreguidelines-init-variables,
Expand All @@ -27,11 +28,14 @@ Checks: >
-cppcoreguidelines-slicing,
-hicpp-explicit-conversions,
-misc-const-correctness,
-misc-include-cleaner,
-misc-redundant-expression,
-misc-misplaced-const,
-misc-confusable-identifiers,
-modernize-avoid-bind,
-modernize-pass-by-value,
-modernize-use-ranges,
-performance-avoid-endl,
-readability-convert-member-functions-to-static,
-readability-implicit-bool-conversion,
-readability-inconsistent-declaration-parameter-name,
Expand All @@ -52,6 +56,7 @@ Checks: >
-bugprone-branch-clone,
-bugprone-easily-swappable-parameters,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-suspicious-stringview-data-usage,
-cppcoreguidelines-avoid-do-while,
-cppcoreguidelines-non-private-member-variables-in-classes,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
Expand All @@ -68,15 +73,19 @@ Checks: >
-modernize-use-trailing-return-type,
-modernize-return-braced-init-list,
-modernize-use-default-member-init,
-modernize-use-designated-initializers,
-modernize-use-nodiscard,
-modernize-use-using,
-portability-simd-intrinsics,
-readability-avoid-return-with-void-value,
-readability-container-data-pointer,
-readability-function-cognitive-complexity,
-readability-identifier-length,
-readability-isolate-declaration,
-readability-math-missing-parentheses,
-readability-non-const-parameter,
-readability-redundant-access-specifiers,
-readability-suspicious-call-argument,
-readability-use-std-min-max,
-readability-use-anyofallof,
---
2 changes: 1 addition & 1 deletion src/cli/tls_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Callbacks : public Botan::TLS::Callbacks {

if(result.successful_validation()) {
output() << "Certificate validation status: " << result.result_string() << "\n";
auto status = result.all_statuses();
const auto& status = result.all_statuses();

if(!status.empty() && status[0].contains(Botan::Certificate_Status_Code::OCSP_RESPONSE_GOOD)) {
output() << "Valid OCSP response for this server\n";
Expand Down
4 changes: 4 additions & 0 deletions src/lib/block/noekeon/noekeon_simd/noekeon_simd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Botan {

namespace {

/*
* Noekeon's Theta Operation
*/
Expand Down Expand Up @@ -55,6 +57,8 @@ inline void gamma(SIMD_4x32& A0, SIMD_4x32& A1, SIMD_4x32& A2, SIMD_4x32& A3) {
A0 ^= A2 & A1;
}

} // namespace

/*
* Noekeon Encryption
*/
Expand Down
4 changes: 4 additions & 0 deletions src/lib/block/serpent/serpent_avx512/serpent_avx512.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

namespace Botan {

namespace {

BOTAN_FORCE_INLINE void SBoxE0(SIMD_16x32& a, SIMD_16x32& b, SIMD_16x32& c, SIMD_16x32& d) {
const SIMD_16x32 t0 = SIMD_16x32::ternary_fn<0xb9>(b, d, c);
const SIMD_16x32 t1 = SIMD_16x32::ternary_fn<0xe2>(a, b, d);
Expand Down Expand Up @@ -261,6 +263,8 @@ BOTAN_FORCE_INLINE void SBoxD7(SIMD_16x32& a, SIMD_16x32& b, SIMD_16x32& c, SIMD
d = o3;
}

} // namespace

BOTAN_AVX512_FN
void Serpent::avx512_encrypt_16(const uint8_t in[16 * 16], uint8_t out[16 * 16]) const {
using namespace Botan::Serpent_F;
Expand Down
4 changes: 4 additions & 0 deletions src/lib/block/threefish_512/threefish_512.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace Botan {

namespace Threefish_F {

namespace {

template <size_t R1, size_t R2, size_t R3, size_t R4>
BOTAN_FORCE_INLINE void e_round(
uint64_t& X0, uint64_t& X1, uint64_t& X2, uint64_t& X3, uint64_t& X4, uint64_t& X5, uint64_t& X6, uint64_t& X7) {
Expand Down Expand Up @@ -142,6 +144,8 @@ BOTAN_FORCE_INLINE void d8_rounds(uint64_t& X0,
key.d_add(R2, X0, X1, X2, X3, X4, X5, X6, X7);
}

} // namespace

} // namespace Threefish_F

void Threefish_512::skein_feedfwd(const secure_vector<uint64_t>& M, const secure_vector<uint64_t>& T) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/ffi/ffi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
namespace Botan_FFI {

// NOLINTNEXTLINE(*-avoid-non-const-global-variables)
thread_local std::string g_last_exception_what;
thread_local static std::string g_last_exception_what;

Check warning on line 25 in src/lib/ffi/ffi.cpp

View workflow job for this annotation

GitHub Actions / Clang Tidy

variable 'g_last_exception_what' declared 'static', move to anonymous namespace instead

int ffi_error_exception_thrown(const char* func_name, const char* exn, int rc) {
g_last_exception_what.assign(exn);
Expand Down
4 changes: 4 additions & 0 deletions src/lib/pubkey/classic_mceliece/cmce_field_ordering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace Botan {

namespace CMCE_CT {

namespace {

template <std::unsigned_integral T1, std::unsigned_integral T2>
requires(sizeof(T1) <= 8 && sizeof(T2) <= 8)
void cond_swap_pair(CT::Mask<uint64_t> cond_mask, std::pair<T1, T2>& a, std::pair<T1, T2>& b) {
Expand Down Expand Up @@ -66,6 +68,8 @@ T min(const T& a, const T& b) {
return mask.select(a, b);
}

} // namespace

} // namespace CMCE_CT

namespace {
Expand Down
8 changes: 4 additions & 4 deletions src/lib/pubkey/mce/mceliece_key.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ std::vector<uint8_t> McEliece_PublicKey::public_key_bits() const {
DER_Encoder(output)
.start_sequence()
.start_sequence()
.encode(static_cast<size_t>(get_code_length()))
.encode(static_cast<size_t>(get_t()))
.encode(get_code_length())
.encode(get_t())
.end_cons()
.encode(m_public_matrix, ASN1_Type::OctetString)
.end_cons();
Expand Down Expand Up @@ -121,8 +121,8 @@ secure_vector<uint8_t> McEliece_PrivateKey::private_key_bits() const {
DER_Encoder enc;
enc.start_sequence()
.start_sequence()
.encode(static_cast<size_t>(get_code_length()))
.encode(static_cast<size_t>(get_t()))
.encode(get_code_length())
.encode(get_t())
.end_cons()
.encode(m_public_matrix, ASN1_Type::OctetString)
.encode(m_g[0].encode(), ASN1_Type::OctetString); // g as octet string
Expand Down
1 change: 1 addition & 0 deletions src/lib/tls/tls13/tls_handshake_layer_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ const T& get(const std::reference_wrapper<T>& v) {

template <typename T>
const T& get(const T& v) {
// NOLINTNEXTLINE(bugprone-return-const-ref-from-parameter)
return v;
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/tls/tls13_pqc/hybrid_public_key.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ std::unique_ptr<Hybrid_KEM_PublicKey> Hybrid_KEM_PublicKey::load_for_group(

BufferSlicer public_value_slicer(concatenated_public_values);
std::vector<std::unique_ptr<Public_Key>> pks;
pks.reserve(alg_ids.size());
for(size_t idx = 0; idx < alg_ids.size(); ++idx) {
pks.emplace_back(load_public_key(alg_ids[idx], public_value_slicer.take(public_value_lengths[idx])));
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/tls/tls_ciphersuite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ bool Ciphersuite::is_usable() const {
return false;
}

const auto mode = cipher_and_mode[1];
const auto& mode = cipher_and_mode[1];

#if !defined(BOTAN_HAS_AEAD_CCM)
if(mode == "CCM" || mode == "CCM-8")
Expand Down
2 changes: 1 addition & 1 deletion src/lib/x509/x509_dn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ std::string X509_DN::to_string() const {
}

std::ostream& operator<<(std::ostream& out, const X509_DN& dn) {
auto info = dn.dn_info();
const auto& info = dn.dn_info();

for(size_t i = 0; i != info.size(); ++i) {
out << to_short_form(info[i].first) << "=\"";
Expand Down
5 changes: 5 additions & 0 deletions src/scripts/dev_tools/run_clang_tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
'misc-confusable-identifiers',
'modernize-avoid-bind',
'modernize-pass-by-value',
'modernize-use-ranges', # limited by compiler support currently
'performance-avoid-endl',
'readability-convert-member-functions-to-static',
'readability-implicit-bool-conversion',
Expand All @@ -85,6 +86,7 @@
'bugprone-branch-clone', # doesn't interact well with feature macros
'bugprone-easily-swappable-parameters',
'bugprone-implicit-widening-of-multiplication-result',
'bugprone-suspicious-stringview-data-usage', # triggers on every use of string_view::data ??
'cppcoreguidelines-avoid-do-while',
'cppcoreguidelines-non-private-member-variables-in-classes', # pk split keys
'cppcoreguidelines-pro-bounds-pointer-arithmetic',
Expand All @@ -101,6 +103,7 @@
'modernize-use-trailing-return-type', # fine, but we're not using it everywhere
'modernize-return-braced-init-list', # thanks I hate it
'modernize-use-default-member-init',
'modernize-use-designated-initializers',
'modernize-use-nodiscard',
'modernize-use-using', # fine not great
'portability-simd-intrinsics',
Expand All @@ -109,9 +112,11 @@
'readability-function-cognitive-complexity',
'readability-identifier-length', # lol, lmao
'readability-isolate-declaration',
'readability-math-missing-parentheses',
'readability-non-const-parameter',
'readability-redundant-access-specifiers', # reneme likes doing this
'readability-suspicious-call-argument',
'readability-use-std-min-max',
'readability-use-anyofallof', # not more readable
]

Expand Down
2 changes: 1 addition & 1 deletion src/tests/test_kyber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ void test_decompress(Test::Result& result) {

result.start_timer();

const auto twotothed = (uint32_t(1) << d);
using from_t = std::conditional_t<d <= 8, uint8_t, uint16_t>;
const from_t twotothed = static_cast<from_t>(from_t(1) << d);

for(from_t y = 0; y < twotothed; ++y) {
const uint32_t c = Kyber_Algos::decompress<d>(y);
Expand Down
2 changes: 1 addition & 1 deletion src/tests/test_pcurves.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Pcurve_Basemul_Tests final : public Text_Based_Test {

if(auto curve = Botan::PCurve::PrimeOrderCurve::from_name(group_id)) {
if(auto scalar = curve->deserialize_scalar(k_bytes)) {
const auto k = scalar.value();
const auto& k = scalar.value();
auto pt2 = curve->mul_by_g(k, rng).to_affine().serialize();
result.test_eq("mul_by_g correct", pt2, P_bytes);

Expand Down
8 changes: 5 additions & 3 deletions src/tests/test_rng_behavior.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,16 +500,18 @@ class HMAC_DRBG_Unit_Tests final : public Stateful_RNG_Tests {
std::vector<uint32_t> security_strengths{128, 192, 256, 256, 256, 256};

for(size_t i = 0; i < approved_hash_fns.size(); ++i) {
std::string hash_fn = approved_hash_fns[i];
std::string mac_name = "HMAC(" + hash_fn + ")";
const auto& hash_fn = approved_hash_fns[i];
const size_t expected_security_level = security_strengths[i];

const std::string mac_name = "HMAC(" + hash_fn + ")";
auto mac = Botan::MessageAuthenticationCode::create(mac_name);
if(!mac) {
result.note_missing(mac_name);
continue;
}

Botan::HMAC_DRBG rng(std::move(mac));
result.test_eq(hash_fn + " security level", rng.security_level(), security_strengths[i]);
result.test_eq(hash_fn + " security level", rng.security_level(), expected_security_level);
}

return result;
Expand Down
Loading

0 comments on commit f24d5af

Please sign in to comment.