wolfcrypt tests: improve file system gating for USE_CERT_BUFFERS#7337
Merged
douzzer merged 1 commit intowolfSSL:masterfrom Mar 30, 2024
Merged
wolfcrypt tests: improve file system gating for USE_CERT_BUFFERS#7337douzzer merged 1 commit intowolfSSL:masterfrom
douzzer merged 1 commit intowolfSSL:masterfrom
Conversation
2adcb1b to
e80f951
Compare
84e4843 to
bf97758
Compare
Contributor
Author
|
Jenkins retest this please |
douzzer
approved these changes
Mar 30, 2024
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.
Description
This PR has been retested (see commit history) and description updated since first submission.
/begin edit:
Former summary:
There's still the oddity of the
USE_CERT_BUFFERS_256defined locally in thewolfcrypt/test/test.cand used by thewolfssl/certs_test.h. This is not longer changed in this PR.Instead, only the
settings.h.is updated here to more robustly handle files orNO_FILESYSTEM.I believe this code does not belong in
test.c, but perhaps should be moved tosettings.has a default, or an error give that the user should select one in the respectiveuser_settings.h:The problem is that the scope of
USE_CERT_BUFFERS_256for this PR is only intest.c, but should be applied throughout wolfcrypt.I've confirmed that removing the
#define USE_CERT_BUFFERS_256is the cause of the current test failures.I propose that the existing tests be modified to take this into account.
Although this PR addresses
USE_CERT_BUFFERS_256, the#define USE_CERT_BUFFERS_2048is probably needs to be similarly addressed./end edit
Similar to #7333, there are likely many nuances of the proposed changes herein that I may be missing.
In short, I think
USE_CERT_BUFFERS_256should be defined in theuser_settings.hand not intest.cwhen needed. Doing this intest.cstill leaves out the ecc definitions in wolfssl/certs_test.h, at least for embedded targets:Of course, pulling that little string and a variety of other related dependencies became unwound, mainly surrounding having a filesystem or not.
I originally found this by plugging the Espressif wolfssl_client user_settings.h into the Espressif wolfssl_test.
There's probably a discussion to be had regarding my proposed revised gating logic, and if
ASN_PARSE_Eis the proper exit code to use.I've also adjusted the indenting to make the code a bit more readable.
Current Behavior
Without the
#define USE_CERT_BUFFERS_256in the above-mentioneduser_settings.h, here are the errors that occurred, and are fixed in this PR.New Behavior
With this update, here's how the missing
USE_CERT_BUFFERS_256error is now presented withDEBUG_WOLFSSLenabled (on the ESP32):Fixes zd# n/a
Testing
How did you test?
Tested primarily in Espressif with various
user_settings.hvalues, as well as a general--enable-allcomandline.Checklist