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

Fix typo with NVStore #6595

Merged
merged 1 commit into from Apr 12, 2018

Conversation

Projects
None yet
6 participants
@ccli8
Contributor

ccli8 commented Apr 11, 2018

Description

I believe the below code is a typo:

mbed-os/features/nvstore/source/nvstore.cpp:

memcpy(_flash_area_params, 0, sizeof(_flash_area_params));

And I fix as below:

memset(_flash_area_params, 0, sizeof(_flash_area_params));

Because _flash_area_params will be re-initialized later, the typo won't cause trouble on most targets. But it will on NUMAKER_PFM_M2351 port of which is on-going. NUMAKER_PFM_M2351 is Cortex-M23 based and has TrustZone support. Read flash address 0 is not always legal because flash is partitioned into secure/non-secure.

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@0xc0170 0xc0170 requested a review from davidsaada Apr 11, 2018

@davidsaada

This line was actually removed in this PR, waiting to be merged. Reason is, as you also noticed, that this structure is initialized at init phase. I don't mind approving this PR, until my PR is merged, if it helps fixing an immediate issue.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 11, 2018

@davidsaada Should this be integrated first as bugfix, thus would be part of the next patch release. And your PR would be for the next minor as adding new API?

@davidsaada

This comment has been minimized.

Contributor

davidsaada commented Apr 11, 2018

Should this be integrated first as bugfix, thus would be part of the next patch release. And your PR would be for the next minor as adding new API?

Yes, I believe it's the right thing to do.

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Apr 11, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 11, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 11, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 12, 2018

@cmonr cmonr merged commit 7f67196 into ARMmbed:master Apr 12, 2018

12 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10159 cycles (+396 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@ccli8 ccli8 deleted the OpenNuvoton:nuvoton_fix_nvstore branch Apr 13, 2018

@adbridge

This comment has been minimized.

Contributor

adbridge commented Apr 20, 2018

This line did not even come into the code base until #6388 which is marked for 5.9 so not sure this should be patched !

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