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

[Backport 2.28] Remove radix argument from bignum test functions #6071

Merged

Conversation

wernerlewis
Copy link
Contributor

Description

Backport of #6070.

Same scripts were ran on 2.28 for most replacements, with manual changes applied as required.

Status

READY

@wernerlewis wernerlewis added enhancement component-crypto Crypto primitives and low-level interfaces component-test Test framework and CI scripts labels Jul 7, 2022
@daverodgman daverodgman added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review priority-high High priority - will be reviewed soon labels Jul 20, 2022
@tom-daubney-arm tom-daubney-arm self-requested a review July 21, 2022 07:47
All uses have radix argument removed, using script.

Signed-off-by: Werner Lewis <[email protected]>
Cases where radix was explictly declared are removed in most cases,
replaced using script. bignum arguments are represented as hexadecimal
strings. This reduces clutter in test data and makes bit patterns
clearer.

Signed-off-by: Werner Lewis <[email protected]>
Functions which are not covered by script, changes made to use radix
16.

Signed-off-by: Werner Lewis <[email protected]>
Test data which is compared as a hex string now uses upper case to
match output of mbedtls_mpi_write_string() output. This removes usage
of strcasecmp().

Signed-off-by: Werner Lewis <[email protected]>
@wernerlewis wernerlewis force-pushed the bignum_test_radix_2.28 branch from 29a8000 to df33684 Compare August 1, 2022 14:57
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

I've verified that the constants that have had their radixes changed have equivalent values, and that those that were originally in hex are unchanged.

Faithful backport.

LGTM

@tom-cosgrove-arm tom-cosgrove-arm removed the needs-reviewer This PR needs someone to pick it up for review label Aug 2, 2022
Copy link
Contributor

@tom-daubney-arm tom-daubney-arm left a comment

Choose a reason for hiding this comment

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

This is a faithful backport of #6070. LGTM

@tom-daubney-arm tom-daubney-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Aug 3, 2022
@tom-cosgrove-arm
Copy link
Contributor

CI was passing, not sure why it failed, so I've re-run the job

@gilles-peskine-arm gilles-peskine-arm merged commit da12621 into Mbed-TLS:mbedtls-2.28 Aug 5, 2022
@wernerlewis wernerlewis deleted the bignum_test_radix_2.28 branch August 5, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-crypto Crypto primitives and low-level interfaces component-test Test framework and CI scripts enhancement priority-high High priority - will be reviewed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants