Change mbedtls_platform_context parameter to NULL #199
Conversation
| int exit_code = MBEDTLS_EXIT_FAILURE; | ||
|
|
||
| if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) { | ||
| if((exit_code = mbedtls_platform_setup(NULL)) != 0) { |
There was a problem hiding this comment.
This is assigning an error code from mbedtls_platform_setup() to an exit_code used to exit main(). They are two different sets of values - and the error values don't map to valid exit codes. Initialise exit_code to MBEDTLS_EXIT_SUCCESS and remove these follow on assignments. They're not needed.
There was a problem hiding this comment.
@sbutcher-arm I disagree.
Although the exit code is not the return code of the functions, we use it for logging the error code. When we exit the application upon failure, the return code will always be MBEDTLS_EXIT_FAILURE
There was a problem hiding this comment.
Well, I disagree. ;) I think it's reusing the variable for two different purposes with sets of values that don't necessarily mix. But it's a pre-existing problem, so I think I can ignore it for the moment, as the code works, just doesn't match my expectations.
There was a problem hiding this comment.
Sure, I created #205 to track this
benchmark/main.cpp
Outdated
| int exit_code = MBEDTLS_EXIT_FAILURE; | ||
|
|
||
| if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) { | ||
| if((exit_code = mbedtls_platform_setup(NULL)) != 0) { |
There was a problem hiding this comment.
Again, I think you should remove the assignment to exit_code, and initialise it to MBEDTLS_EXIT_SUCCESS instead.
| int exit_code = MBEDTLS_EXIT_FAILURE; | ||
|
|
||
| if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) { | ||
| if((exit_code = mbedtls_platform_setup(NULL)) != 0) { |
There was a problem hiding this comment.
Same review comment as before - remove the assignment toexit_code, and initialise it to MBEDTLS_EXIT_SUCCESS instead.
| int exit_code = MBEDTLS_EXIT_FAILURE; | ||
|
|
||
| if((exit_code = mbedtls_platform_setup(&platform_ctx)) != 0) { | ||
| if((exit_code = mbedtls_platform_setup(NULL)) != 0) { |
There was a problem hiding this comment.
Same comment as above - remove the assignment to exit_code.
|
@sbutcher-arm I answered your comment. |
|
@k-stachowiak I agree that perhaps it's not the best approach, however I think that having a new variable ( |
|
@RonEld - Can you please rebase this and resolve the conflicts? |
RonEld
force-pushed
the
set_NULL_as_platform_context
branch
from
e3bb3b8 to
d47269d
Compare
|
@sbutcher-arm I have rebased to resolve the conflicts |
|
retest |
|
@RonEld - I hate to ask - but can you please rebase? |
`mbedtls_platform_setup()` and `mbedtls_platform_teardown()` now ignore the context parameter, since #mbed-os/7099 was merged. Changing the sent parameter to NULL, to avoid misleading.
RonEld
force-pushed
the
set_NULL_as_platform_context
branch
from
d47269d to
9fff2bb
Compare
|
@sbutcher-arm Done. Please review |
|
retest |
|
CI is failing, but only with known issues (of which we have far too many!), so this can be merged. |
mbedtls_platform_setup()andmbedtls_platform_teardown()now ignore the context parameter, since ARMmbed/mbed-os#7099 was merged. Changing the sent parameter to NULL, to avoid misleading.