apps/speed: merge curve list definitions#6132
apps/speed: merge curve list definitions#6132FdaSilvaYY wants to merge 3 commits intoopenssl:masterfrom
Conversation
|
Red cross from appveyor is from TLSProxy tests and caused by server resetting connection before alert went through "wire". This is unrelated. |
apps/speed.c
Outdated
There was a problem hiding this comment.
It doesn't actually have to typedef, but ok...
apps/speed.c
Outdated
There was a problem hiding this comment.
I wish it was possible to omit EC_NUM. But at least, while you are at it, would you declare this array as [], and insert OPENSSL_assert that compares number of elements to EC_NUM? So that it works as remainder reminder when you add things to it...
There was a problem hiding this comment.
Oups, will looking at this more carefully, I found a larger issue.
There is :
- 18 ecdh curves as EC_NUM states
- and only ecdsa 16 curves but it reuse EC_NUM value
Fix in progress => see #6133
FdaSilvaYY
force-pushed
the
apps/speed/merge_curve_lists
branch
2 times, most recently
from
0f4de48 to
5e820b7
Compare
|
Fixed and rebase; CI's are fully Green 🍾 |
apps/speed.c
Outdated
There was a problem hiding this comment.
Hmm, I was under impression that preferred style would be to have opening { at same line. See for example ecp_nistp256.c...
There was a problem hiding this comment.
It's minor formatting nit, so I'm approving this provided that fix-up would be a separate commit that would be squashed upon commit to repo.
dot-asm
added
the
approval: review pending
FdaSilvaYY
force-pushed
the
apps/speed/merge_curve_lists
branch
from
5e820b7 to
e28f506
Compare
|
Hmmm... As procedural comment. "Provided that fix-up would be a separate commit" was actually condition that it was commit on top [of] prior commits, more specifically with their original ids. But now all commit |
levitte
added
approval: done
|
Editorial note. I'm going to squash everything into one commit. |
|
@dot-asm : sorry for the rebase, it was a reflex ;) |
|
Merged. Thanks! |
... and unify 'bits' declarations and printing format. Reviewed-by: Andy Polyakov <[email protected]> Reviewed-by: Richard Levitte <[email protected]> (Merged from #6132)
3 participants
It's a small code improvement: just merge the three list of values declaring each elliptic curve.
Edit : Second commit is because other
bitsare defined as unsigned except test_curves->bits.