Skip to content
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

Prevent dead code warning #159

Conversation

k-stachowiak
Copy link
Contributor

@k-stachowiak k-stachowiak commented Jul 3, 2019

The window size variable in ecp_pick_window_size() can take values 4, 5 or 6, but we clamp it not to exceed the value of MBEDTLS_ECP_WINDOW_SIZE. If that is 6 (default) or higher, the
static analyzer will point out that the test: w > MBEDTLS_ECP_WINDOW_SIZE always evaluates to false.

This commit removes the test for the cases of the window size large enough to fit all the potential values of the variable.

The issue was found by Coverity and fixed the same way @pkolbus has fixed a similar problem in: Mbed-TLS/mbedtls#2317

Copy link
Collaborator

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavior change

library/ecp.c Outdated Show resolved Hide resolved
The window size variable in ecp_pick_window_size() can take values
4, 5 or 6, but we clamp it not to exceed the value of
MBEDTLS_ECP_WINDOW_SIZE. If that is 6 (default) or higher, the
static analyzer will point out that the test:
w > MBEDTLS_ECP_WINDOW_SIZE always evaluates to false.

This commit removes the test for the cases of the window size
large enough to fit all the potential values of the variable.
@k-stachowiak k-stachowiak force-pushed the IOTCRYPT-474-prevent-dead-code-warning branch from 8a28052 to 653a4a2 Compare July 4, 2019 10:51
@Patater Patater merged commit e78cd62 into ARMmbed:development Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants