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.

Sign up for GitHub

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

Jump to bottom

Small fixes for SecureStore #11988

Merged
merged 6 commits into from
Dec 5, 2019
Merged

Small fixes for SecureStore #11988

merged 6 commits into from
Dec 5, 2019

Conversation

SeppoTakalo
Copy link
Contributor

Description

Summary of change

  • SecureStore: Deinitialize also member TDBStorages on deinit()
  • SecureStore: Validate internal RBP data first
  • SecureStore: Don't use flags of corrupted data. Allow removing.
  • SecureStore: Validate internal header size before using its values.

Documentation

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@VeijoPesonen

Release Notes

Summary of changes

Impact of changes

Migration actions required

VeijoPesonen
VeijoPesonen approved these changes Nov 29, 2019
View reviewed changes
Copy link
Contributor

@VeijoPesonen VeijoPesonen left a comment

Choose a reason for hiding this comment

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

Looks good to me

@ciarmcom ciarmcom requested review from VeijoPesonen and a team November 29, 2019 12:00
@ciarmcom
Copy link
Member

@SeppoTakalo, thank you for your changes.
@VeijoPesonen @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@mbed-ci
Copy link

mbed-ci commented Nov 29, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

Seppo Takalo added 4 commits December 4, 2019 14:55
@SeppoTakalo SeppoTakalo force-pushed the feature_securestore_refactoring branch from db166cf to 68d2f8a Compare December 4, 2019 12:59
@0xc0170
Copy link
Contributor

0xc0170 commented Dec 4, 2019

Also please fix travis astyle

@SeppoTakalo SeppoTakalo force-pushed the feature_securestore_refactoring branch 3 times, most recently from 5d450e8 to 7fa68ea Compare December 4, 2019 14:17
@SeppoTakalo SeppoTakalo force-pushed the feature_securestore_refactoring branch from 7fa68ea to 21acb66 Compare December 4, 2019 14:22
@SeppoTakalo
Copy link
Contributor Author

Changed the (type *) to reinterpret_cast<type *>

@mbed-ci
Copy link

mbed-ci commented Dec 4, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@mbed-ci
Copy link

mbed-ci commented Dec 4, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Dec 5, 2019
@0xc0170 0xc0170 merged commit e2a11c0 into master Dec 5, 2019
@0xc0170 0xc0170 deleted the feature_securestore_refactoring branch December 5, 2019 07:23
@adbridge
Copy link
Contributor

@0xc0170 as this is marked as feature update, shouldn't it have release notes and documentation sections filled in ? @bulislaw as feature updates may contain new APIs but are technically not breaking changes do we still want Christos to consider these in his migration guide?

@SeppoTakalo
Copy link
Contributor Author

This is more like a internal refactoring work that touched so big sections that we did not want to push out into patch release.
API did not change.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 17, 2020

Thanks Seppo, this one should be fine as it is.

@bulislaw
Copy link
Member

Each new feature should have release notes, but as Seppo said it's a refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kjbracey kjbracey kjbracey approved these changes

@VeijoPesonen VeijoPesonen VeijoPesonen approved these changes

Assignees
No one assigned
Labels
release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@SeppoTakalo @ciarmcom @mbed-ci @0xc0170 @adbridge @bulislaw @kjbracey @VeijoPesonen