Updates to align with name changes and corrections in wolfHSM PR52 #12
Conversation
billphipps
force-pushed
the
update_with_main_lib
branch
from
eb084af
to
84aafeb
Compare
| $(WOLFSSL_DIR)/wolfcrypt/src/sha256.c \ | ||
| $(WOLFSSL_DIR)/wolfcrypt/src/aes.c \ | ||
| $(WOLFSSL_DIR)/wolfcrypt/src/cmac.c | ||
| SRC_C += $(wildcard $(WOLFSSL_DIR)/wolfcrypt/src/*.c) |
There was a problem hiding this comment.
Hmmm I don't like wildcards in makefiles - can make for some hard to diagnose issues. Even though it takes up space, I think we should explicitly add all the source files we want to compile. Also serves customers better as they can see exactly which files are required to build, in case they are trying to replicate a minimal example in a different build environment. Just food for thought.
There was a problem hiding this comment.
Let's make wildcard an option in the Makefile, not selected by default. Using the wildcard means we can swap out different user settings without touching the makefile. Also, wildcards makes sure that IDE's won't have an issue. Note that I only included the C files and not the wolfcrypt/port files. I'll tweak to make it optional.
|
|
||
| #include "wolfcrypt/test/test.h" | ||
|
|
||
| #include "wh_demo_client_counter.h" |
| int wh_DemoClient_wcTest(whClientContext* clientContext) | ||
| { | ||
| return wolfcrypt_test(NULL); | ||
| } |
There was a problem hiding this comment.
We should probably write something documenting that the caller is responsible for initializing wolfCrypt, RNG, etc before invoking this function, and tearing down after. On one hand, I want a demo function that includes doing everything it needs to demonstrate how a user would do it, but on the other hand the wolfCrypt init is global, so would screw things up for other demos if it deinits wolfCrypt. IDK. Maybe just write something indicating that wolfCrypt needs to be initialized along with the client context before invoking the functions
There was a problem hiding this comment.
I'll add some comments. IMHO, wc_CryptoCbRegister/Deregister should handle the reference counting. Mayhaps I'll put together a mainline PR...
| /* Math library selection. */ | ||
| #define USE_FAST_MATH |
There was a problem hiding this comment.
Should probably be using SP math for our official demos, since that is our preferred option
There was a problem hiding this comment.
Oops. I'll remove that to use the default. Was troubleshooting.
| if (ret != 0) { | ||
| printf("Failed to wc_AesInit %d\n", ret); | ||
| goto exit; |
There was a problem hiding this comment.
I'd say either scrub all the gotos in the file like you did in this function, or leave them as is, but probs don't want a mix
There was a problem hiding this comment.
Yup. more cleanups are a-coming!
Aligning with wolfSSL/wolfHSM#52. Draft until that is merged.