-
Notifications
You must be signed in to change notification settings - Fork 133
Improve user settings detection #635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Improve user settings detection #635
Conversation
danielinux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, except a few minor things. I did not check the changes affecting win32.
1167316 to
2875939
Compare
2875939 to
dfa6c7e
Compare
| #endif | ||
|
|
||
| #include "wolfboot/wolfboot.h" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this section, do not touch otp-keystore-gen tool, as discussed. No mixing user_settings.h is happening here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I previously misunderstood; I thought it was only the primer: otp-keystore-primer.c to leave alone.
I'll also leave alone otp-keystore-gen.c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll close #634 that is only related to otp.
|
|
||
| /* Some SHA checks */ | ||
| #if !defined(WOLFBOOT_SHA_DIGEST_SIZE) || (WOLFBOOT_SHA_DIGEST_SIZE <= 0) | ||
| # error "WOLFBOOT_SHA_DIGEST_SIZE must be defined" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually redundant. The check is performed in wolfboot.h . See error at wolfboot.h:225
| #endif | ||
| #endif /* optional user settings check */ | ||
|
|
||
| /* Some debug options. See docs. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be proper options via tools/config.mk, parsed in the tools makefile
| /* Must also define DEBUG_WOLFSSL in user_settings.h */ | ||
| //#define DEBUG_SIGNTOOL | ||
| /* Must also define DEBUG_WOLFSSL in /tools/keytools/user_settings.h */ | ||
| /* #define DEBUG_SIGNTOOL */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a proper options via tools/config.mk, parsed via tools/keytools/Makefile. You can have a separate "config" in your IDE magic perhaps for first run, while Makefile users can add DEBUG_SIGNTOOL=1 in .config
user_settings.h is not meant for being modified.
Improve detection at compile time for which
user_settings.hfile is being referenced.Also adds some minor code formatting cleanup, some MSVC-specific changes.
Builds on the changes in #624