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

Address various warnings new in clang-tidy 19 #4548

Merged
merged 1 commit into from
Jan 17, 2025
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
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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to enable and act on that as soon as we have the compilers for it. Perhaps leave a TODO(Botan4) or something? Edit: oh, right. I found the comment in the other list.

-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
4 changes: 4 additions & 0 deletions src/lib/ffi/ffi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@

namespace Botan_FFI {

namespace {

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

} // namespace

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
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
Loading