Improve robustness and testing of mbedtls_mpi_copy #346

2020-01-20T14:23:23-06:00

gilles-peskine-arm commented

2020-01-20 14:23:23 -06:00

(Migrated from github.com)

Also improve testing of mbedtls_mpi_swap.

The robustness improvement could plausibly fix a bug in some applications, so this should be backported.

Also improve testing of `mbedtls_mpi_swap`. The robustness improvement could plausibly fix a bug in some applications, so this should be backported.

mpg (Migrated from github.com) reviewed

2020-01-21 04:44:22 -06:00

mpg (Migrated from github.com) left a comment

No description provided.

Looks good to me. I just left a suggestion for improving a comment, which you may choose to ignore as perhaps my initial confusion would be best fixed by me getting more sleep than by rewording the comment.

mpg commented

2020-01-21 05:02:27 -06:00

(Migrated from github.com)

I checked the CI results, and the only failures are in Mbed OS testing, which is expected as Mbed OS needs updating, so it's as good as a pass.

mpg (Migrated from github.com) reviewed

2020-01-22 02:25:44 -06:00

mpg (Migrated from github.com) left a comment

No description provided.

LGTM. Thanks for improving the comment.

mpg commented

2020-01-22 02:27:19 -06:00

(Migrated from github.com)

For some reason the CI had directly reported failure without actually running any test. I'm restarting it.

yanesca (Migrated from github.com) requested changes

2020-02-03 08:55:11 -06:00

yanesca (Migrated from github.com) left a comment

I have two minor suggestions for improvement.

tests/suites/test_suite_mpi.data

					
				@ -292,37 +205,106 @@ mbedtls_mpi_cmp_abs:10:"2":10:"-3":-1

				Base test mbedtls_mpi_cmp_abs (Mix values) #3

I am not sure I understand this description, could you please make it slightly more verbose?

tests/suites/test_suite_mpi.function

					
				@ -629,22 +604,40 @@ exit:

				/* END_CASE */

Our existing tests are using big endian input and I think it would be less confusing to have consistency in this matter. Why do you prefer little endian by the way?