-
Notifications
You must be signed in to change notification settings - Fork 923
WOLFSSL_SP_INT_NEGATIVE declaration for all Espressif chipsets #6374
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
Conversation
|
The Jenkins error is unrelated to the changes in this PR: |
| int ret = 0; | ||
|
|
||
| #ifdef WOLFSSL_SP_INT_NEGATIVE | ||
| int neg = (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; |
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 recommend this just be int neg and keep the existing setting of it in CONFIG_IDF_TARGET_ESP32S3. The !CONFIG_IDF_TARGET_ESP32S3 case overrides this with a different value.
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.
Good point on the code inconsistency. I left the declaration & assignment at the top and instead moved the Z->sign logic completely out of the IF/CONFIG sections.
The negative flag setting is now assigned regardless of IDF_TARGET_[n] value at the end of the function as updated in 2ec486e.
|
|
||
| #ifdef WOLFSSL_SP_INT_NEGATIVE | ||
| /* neg check: X*Y becomes negative */ | ||
| int neg = (mp_isneg(X) == mp_isneg(Y)) ? MP_ZPOS : MP_NEG; // (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; |
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.
C style /* */ comments please.
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.
erg. small monitor. miss that. fixed, thanks.
|
I've updated the original description to include a note regarding an additional minor but critical change to esp32_sha.c needed when |
dgarske
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.
Changes look good.
…SL#6374) * WOLFSSL_SP_INT_NEGATIVE declaration for all Espressif chipsets * correct naming for WOLFSSL_SHA384 on ESP32-C3
Description
This minor PR changes the scope of the
negvariable to be declared for all Espressif chipsets, not just the ESP32-S3.Fixes #6373
See
Fixes zd# n/a
Testing
How did you test?
Ran the wolfssl_test for the ESP32 with
#define OPENSSL_ALLinuser_settings.hChecklist