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

Change mbedtls_platform_context parameter to NULL #7960

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

RonEld
Copy link
Contributor

@RonEld RonEld commented Sep 3, 2018

Description

mbedtls_platform_setup() and mbedtls_platform_teardown() now ignore the context parameter, since #7099 was merged. Changing the sent parameter to NULL, to avoid misleading.
This PR continues #7099

Pull request type

[ ] Fix
[x] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

Change the parameter being sent to `mbedtls_platform_setup()` and
`mbedtls_platfrm_teardown()` to NULL, as it is now being unused
in Mbed OS.
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 3, 2018

@RonEld Who should review these changes?

@RonEld
Copy link
Contributor Author

RonEld commented Sep 3, 2018

@RonEld Who should review these changes?

I would say that @pan- should review the BLE part and @sbutcher-arm , @Patater or someone on their behalf should review the Mbed TLS part

@0xc0170 0xc0170 requested review from pan- and Patater September 3, 2018 09:06
Copy link
Contributor

@simonbutcher simonbutcher left a comment

Choose a reason for hiding this comment

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

We really need, in addition to these changes, a test to test out the reference counting of the resource. Two stacked setup's, followed by a crypto test operation a teardown, another test operation and a teardown, or similar.

Not a blocker - so @RonEld - can you either add that test, or create a JIRA ticket and the team can do it at a later date.

@RonEld
Copy link
Contributor Author

RonEld commented Sep 4, 2018

@sbutcher-arm

Not a blocker - so @RonEld - can you either add that test, or create a JIRA ticket and the team can do it at a later date.

Created IOTSSL-2502 to track this

Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

Good for me on the BLE side.

@RonEld
Copy link
Contributor Author

RonEld commented Sep 4, 2018

@0xc0170 Since this PR was reviewed by both @pan- and @sbutcher-arm , I think we can remove review request from @Patater
In addition, could you please start the CI? Although this is not a blocker for 5.10 , I would like to see it in 5.10,as a follow up of #7099 , unless it won't make it

@kjbracey
Copy link
Contributor

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 10, 2018

Build : SUCCESS

Build number : 3043
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7960/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Sep 10, 2018

@mbed-ci
Copy link

mbed-ci commented Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants