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.

Sign up for GitHub

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

Jump to bottom

Is this line still needed? #2377

Closed
irwir opened this issue Jan 26, 2019 · 5 comments · Fixed by ARMmbed/mbed-crypto#225
Closed

Is this line still needed? #2377

irwir opened this issue Jan 26, 2019 · 5 comments · Fixed by ARMmbed/mbed-crypto#225
Labels
bug component-crypto Crypto primitives and low-level interfaces

Comments

@irwir
Copy link
Contributor

irwir commented Jan 26, 2019

Question

https://github.com/ARMmbed/mbedtls/blob/11cdb0559e46dbe9301bd37a36d583e3d2fefd3b/library/bignum.c#L2104

The i value was used for calculating the upper limit of the next for loop.
Currently (since the commit b9eb786) this limit is passed as a function parameter.
@RonEld
Copy link
Contributor

RonEld commented Jan 27, 2019

@irwir Thank you for your question.
I am not sure I follow you, i in this line in miller-rabin is still used in the next for loop , and not given as a function parameter.

@irwir
Copy link
Contributor Author

irwir commented Jan 27, 2019 •
edited
Loading

@RonEld

I am not sure I follow you,

The code quote was taken from the old commit. Please compare with the current code.

@RonEld
Copy link
Contributor

RonEld commented Jan 27, 2019

@irwir I was looking at the current code, and i is still used in the for loop.
However, I see what you mean, as it is initiated with 0, thus I believe you are correct and the line is redundant.
Thank you for raising this issue!

@RonEld RonEld added bug tracking component-crypto Crypto primitives and low-level interfaces and removed question labels Jan 27, 2019
@irwir
Copy link
Contributor Author

irwir commented Jan 27, 2019

Yes, the value is not used and the calculation had no side effects. The variable is in use,, but I did not suggest to remove the declaration.

@ciarmcom
Copy link

ARM Internal Ref: IOTSSL-2739

RonEld pushed a commit to RonEld/mbedtls that referenced this issue Apr 14, 2019
Remove a redundant function call
c46a466 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
@RonEld RonEld mentioned this issue Apr 14, 2019
Closed
4 tasks
RonEld pushed a commit to RonEld/mbedtls that referenced this issue Apr 14, 2019
Remove a redundant function call
7bad964 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
@RonEld RonEld added needs-backports Backports are missing or are pending review and approval. fix available and removed needs-backports Backports are missing or are pending review and approval. labels Apr 14, 2019
RonEld pushed a commit to RonEld/mbedtls that referenced this issue May 1, 2019
Remove a redundant function call
c16baf5 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
RonEld pushed a commit to RonEld/mbedtls that referenced this issue May 1, 2019
Remove a redundant function call
0523e28 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
RonEld pushed a commit to RonEld/mbedtls that referenced this issue May 1, 2019
Remove a redundant function call
ef563c9 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
RonEld pushed a commit to RonEld/mbedtls that referenced this issue May 1, 2019
Remove a redundant function call
ff8d8d7 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
@RonEld RonEld mentioned this issue Aug 18, 2019
Merged
gilles-peskine-arm pushed a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 3, 2019
Remove a redundant function call
c95d9ee 
Remove a call to `mbedtls_mpi_bitlen()` since the returned value is
overwritten in the line after. This is redundant since da31fa1.
Fixes Mbed-TLS#2377.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Milestone
No milestone
3 participants
@ciarmcom @irwir @RonEld