fix: move ERR_error_string funcs out of internal.c (ZD-21735)#10369
Open
MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
Open
fix: move ERR_error_string funcs out of internal.c (ZD-21735)#10369MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
Conversation
wolfSSL_ERR_error_string, wolfSSL_ERR_error_string_n, wolfSSL_ERR_reason_error_string, and SetErrorString were defined in src/internal.c and src/ssl.c, both of which are excluded from WOLFCRYPT_ONLY builds by the build system. This caused link failures when building with --enable-cryptonly --enable-opensslextra: the symbols were declared in wolfssl/ssl.h but not present in libwolfssl.a. Move all four functions (plus the static OpenSSL-compat helper wolfSSL_ERR_reason_error_string_OpenSSL) to wolfcrypt/src/error.c, which is compiled in all wolfSSL configurations including WOLFCRYPT_ONLY. Closes ZD-21735.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes --enable-cryptonly --enable-opensslextra link failures by ensuring OpenSSL-compat error-string APIs are built into crypto-only configurations (where src/internal.c and src/ssl.c are excluded by the build).
Changes:
- Move
wolfSSL_ERR_error_string,wolfSSL_ERR_error_string_n,wolfSSL_ERR_reason_error_string,SetErrorString, and the OpenSSL-reason helper intowolfcrypt/src/error.c(compiled in all configurations). - Remove duplicate/now-relocated
wolfSSL_ERR_error_string*implementations fromsrc/ssl.c. - Remove the SSL/TLS-range reason-string implementation and
SetErrorStringfromsrc/internal.c.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
wolfcrypt/src/error.c |
Adds SSL/TLS-range error-string functions so the symbols exist in WOLFCRYPT_ONLY + OPENSSL_EXTRA builds. |
src/ssl.c |
Removes wolfSSL_ERR_error_string / _n implementations that are now provided by wolfcrypt/src/error.c. |
src/internal.c |
Removes SSL/TLS-range reason-string implementation and SetErrorString now hosted in wolfcrypt/src/error.c. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10369
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL_ERR_error_string, wolfSSL_ERR_error_string_n, wolfSSL_ERR_reason_error_string, and SetErrorString were placed inside wolfcrypt/src/error.c's top-level #ifndef NO_ERROR_STRINGS block, making them invisible when NO_ERROR_STRINGS is defined — the same breakage as the original internal.c placement. Move all five functions (including the static OpenSSL helper) to after the #endif, outside the guard. The function bodies already handle NO_ERROR_STRINGS internally where needed. Also remove a spurious top-level #ifdef WOLFSSL_DEBUG_TRACE_ERROR_CODES_H / debug-untrace-error-codes.h block that was incorrectly placed at file scope (it originated from the internal function body in internal.c). Addresses Fenrir review comment on PR wolfSSL#10369.
Member
|
@MarkAtwood please fix merge conflicts. Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause
wolfSSL_ERR_error_string,wolfSSL_ERR_error_string_n,wolfSSL_ERR_reason_error_string, andSetErrorStringwere defined insrc/internal.candsrc/ssl.c. Both files are excluded fromWOLFCRYPT_ONLYbuilds by the build system —src/include.amgates them onif !BUILD_CRYPTONLY, so neither is compiled or linked when building with--enable-cryptonly.The symbols are declared in
wolfssl/ssl.h(visible to callers) but absent fromlibwolfssl.ain a crypto-only build, causing undefined symbol link errors for any application calling these functions.Why Was This Not Caught Earlier
The
WOLFCRYPT_ONLY + OPENSSL_EXTRAcombination is an edge case not covered by common CI configurations. The placement ininternal.cdates to December 2013 (commit a36c18c), when an external contributor addedCyaSSL_ERR_reason_error_string. It was put ininternal.cbecauseSetErrorStringalready lived there. Three subsequent major refactors never revisited the placement.Fix
Move all five functions (including the static
wolfSSL_ERR_reason_error_string_OpenSSLhelper) towolfcrypt/src/error.c, which is compiled in all wolfSSL configurations: full TLS, WOLFCRYPT_ONLY, and FIPS.The functions are placed after the file-level
#endif /* !NO_ERROR_STRINGS */so they are unconditionally compiled regardless ofNO_ERROR_STRINGS. The function bodies already handleNO_ERROR_STRINGSinternally where needed (wolfSSL_ERR_reason_error_stringreturns a stub string in that case).Added unconditional
#include <wolfssl/error-ssl.h>and#include <wolfssl/ssl.h>before the new block. Several other wolfcrypt port files already includewolfssl/error-ssl.h, so this is an established pattern.Also removed a spurious top-level
#ifdef WOLFSSL_DEBUG_TRACE_ERROR_CODES_H/ debug-untrace block that was incorrectly placed at file scope during extraction frominternal.c.Testing
testwolfcryptpasses,unit.testpasses--enable-cryptonly --enable-opensslextra: library builds cleanly;nm libwolfssl.aconfirms all five symbols present inerror.oFixes ZD-21735.