Skip to content

warnings: silence Botan FFI deprecations + policy doc (#08) - #2428

Closed
ronaldtse wants to merge 1 commit into
mainfrom
compile-warnings
Closed

warnings: silence Botan FFI deprecations + policy doc (#08)#2428
ronaldtse wants to merge 1 commit into
mainfrom
compile-warnings

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

First PR for roadmap item #8 (compile-time warnings clean baseline). Brings the Botan debug build to zero warnings under -Wall -Wextra and documents the warnings policy going forward.

What's in this PR

  • src/lib/crypto/ec.cpp + src/lib/crypto/eddsa.cpp: Three call sites use Botan FFI accessors deprecated in Botan 3.6 (botan_privkey_x25519_get_privkey, botan_pubkey_x25519_get_pubkey, botan_privkey_ed25519_get_privkey). The non-deprecated replacement botan_privkey_view_raw / botan_pubkey_view_raw was added in Botan 3.6 — rnp supports Botan 2.14+, so we can't migrate yet. Each call is wrapped in #pragma GCC diagnostic push/pop with a comment explaining the constraint.
  • docs/develop/compile-time-warnings.adoc (new): Policy for new code. Categorises warnings (real bug vs stylistic vs platform quirk), specifies when to fix vs when to suppress, and requires every suppression to state the warning, why it can't be fixed, and what would remove the suppression.

Why

A clean compile baseline is the prerequisite for -Werror on main CI. Warnings accumulate noise that hides real bugs; the goal is "the warning you just introduced gets your PR blocked". This PR doesn't enable -Werror — that lands once every warning category is addressed. Following the roadmap's guidance, this PR covers exactly one category (Botan FFI deprecations); subsequent PRs will handle other categories (unused parameters, signed/unsigned, MSVC-specific).

Current warning baseline

Built locally with -Wall -Wextra -Wno-unused-parameter -DCMAKE_BUILD_TYPE=Debug:

Backend Warnings
Botan 3.12 (macOS) 0 (was 3, all Botan FFI deprecations)
OpenSSL 3.x (macOS) 0

Out of scope (follow-up PRs)

  • Enable -Werror on a CI leg
  • Audit warnings under gcc older versions, clang older versions, MSVC v143, ClangCL
  • Other warning categories: unused-parameter (currently suppressed with -Wno-unused-parameter), signed/unsigned comparisons, narrowing conversions, MSVC-specific deprecations
  • Migrate to botan_privkey_view_raw (requires dropping Botan 2.x and 3.0–3.5 support)

Test plan

  • Local Botan 3.12 build with -Wall -Wextra -Wno-unused-parameter produces zero warnings (was 3)
  • Local OpenSSL 3.x build with the same flags produces zero warnings
  • Test suite still passes (rnp_tests runs)
  • CI green on all platforms

Three call sites (x25519 secret/public-key gen, ed25519 secret-key gen)
use Botan FFI accessors deprecated in Botan 3.6 in favour of
botan_privkey_view_raw / botan_pubkey_view_raw. rnp supports Botan 2.14+,
so we cannot migrate to the new API yet. Suppress the warning locally
with pragma GCC diagnostic push/pop and a comment explaining the
constraint. This brings the Botan build to zero warnings under
-Wall -Wextra.

Adds docs/develop/compile-time-warnings.adoc documenting the
categorisation (real bug vs stylistic vs platform quirk) and the
required form of a suppression (warning, why, what would remove it).
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.37%. Comparing base (975b67c) to head (f4b3263).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2428      +/-   ##
==========================================
- Coverage   85.37%   85.37%   -0.01%     
==========================================
  Files         126      126              
  Lines       22789    22788       -1     
==========================================
- Hits        19457    19456       -1     
  Misses       3332     3332              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ronaldtse
ronaldtse requested review from antonsviridenko and ni4 July 31, 2026 16:20
@ronaldtse

Copy link
Copy Markdown
Contributor Author

Closing — superseded by #2448 which migrates Botan backend to FFI, resolving the deprecation warnings this PR was silencing. Combined to reduce review overhead.

@ronaldtse ronaldtse closed this Aug 5, 2026
ronaldtse added a commit that referenced this pull request Aug 5, 2026
…s PR

Merges the -Wnonnull-compare fix (RNP_LOG_KEY_NN macro + rnp_key_store
usage) and the compile-time-warnings.adoc policy doc from the now-closed
#2446 and #2428 into this PR, since the FFI migration naturally resolves
the deprecation warnings those PRs were patching.

Reduces review overhead: one PR covers the FFI migration + the warning
policy + the nonnull-compare fix.
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.

1 participant