Fix compiling when WOLFSSH_NO_NISTP256_MLKEM768_SHA256 is defined.#887
Fix compiling when WOLFSSH_NO_NISTP256_MLKEM768_SHA256 is defined.#887tjko wants to merge 1 commit intowolfSSL:masterfrom
Conversation
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Pull request overview
Fixes a compile-time guard so KEXKEM message IDs remain available when only one of the ML-KEM768 SHA256 KEX variants is enabled (e.g., NIST P-256 disabled but Curve25519 still enabled), addressing build failures reported in #886.
Changes:
- Update
#ifndef WOLFSSH_NO_NISTP256_MLKEM768_SHA256to a combined preprocessor condition that accounts for both NISTP256 and Curve25519 ML-KEM768 SHA256 KEX options. - Apply the same guard update for both
MSGID_KEXKEM_INITandMSGID_KEXKEM_REPLY.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfssh/internal.h
Outdated
| #if !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256) || \ | ||
| !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256) |
There was a problem hiding this comment.
This condition is correct but harder to read/maintain due to the negated OR and line continuation. Consider rewriting it into a more self-explanatory form (e.g., negating a combined defined() expression) or centralizing it behind a single helper macro (e.g., WOLFSSH_HAVE_KEXKEM) to avoid duplicating the same multi-line condition in multiple places.
wolfssh/internal.h
Outdated
| #if !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256) || \ | ||
| !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256) |
There was a problem hiding this comment.
The same combined preprocessor condition is duplicated here. To reduce duplication and the chance of the two guards drifting over time, consider extracting the condition into a single macro (or reusing the same macro suggested above) and referencing it in both locations.
|
Btw, seems like this check should also include |
|
Could you remove the two guard checks and then squash the PR? |
See #886