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

Corrected casting issue with an assert within the error_handling test #7202

Merged
merged 3 commits into from
Jun 15, 2018
Merged

Corrected casting issue with an assert within the error_handling test #7202

merged 3 commits into from
Jun 15, 2018

Conversation

cmonr
Copy link
Contributor

@cmonr cmonr commented Jun 13, 2018

Description

Discovered via #7105.
If both values are negative values, they are casted in such a way that -1 != -1. This small commit fixes that.

Pull request type

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

@cmonr
Corrected casting issue with an assert within the error_handling test
6fa5609 
Discovered via #7105.
If both values are negative values, they are casted in such a way that -1 != -1. This small commit fixes that.
@cmonr cmonr requested a review from SenRamakri June 13, 2018 03:10
0xc0170
0xc0170 previously approved these changes Jun 13, 2018
View reviewed changes
0xc0170
0xc0170 reviewed Jun 13, 2018
View reviewed changes
TESTS/mbed_platform/error_handling/main.cpp
@@ -124,7 +124,7 @@ void test_error_context_capture()
mbed_error_status_t status = mbed_get_last_error_info( &error_ctx );
TEST_ASSERT(status == MBED_SUCCESS);
TEST_ASSERT_EQUAL_UINT(error_value, error_ctx.error_value);
TEST_ASSERT_EQUAL_UINT(osThreadGetId(), error_ctx.thread_id);
TEST_ASSERT_EQUAL_UINT((uint32_t)osThreadGetId(), error_ctx.thread_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Change looks fine.

However looking at the implementation because of this bugfix:

osRtxThread_t *current_thread = osRtxInfo.thread.run.curr; , osRtxInfo should not be used - internal detail? Use rather osRtxThreadGetRunning, but that is also I do not believe public API. Will check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SenRamakri ^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xc0170 Addressed with latest commit.

SenRamakri
SenRamakri previously approved these changes Jun 13, 2018
View reviewed changes
@cmonr
Wrapped test around MBED_CONF_RTOS_PRESENT
5fe241b 
Wrapping a #define to prevent test from being run when no RTOS is present.
@cmonr cmonr dismissed stale reviews from SenRamakri and 0xc0170 via 5fe241b June 13, 2018 14:28
0xc0170
0xc0170 previously approved these changes Jun 13, 2018
View reviewed changes
@cmonr
Copy link
Contributor Author

cmonr commented Jun 13, 2018

/morph build

@cmonr
Only exclude RTOS-specifc tests in error_handling
e93aaab 
Allows for other tests to run if no RTOS is present.
@cmonr
Copy link
Contributor Author

cmonr commented Jun 13, 2018

@0xc0170 @SenRamakri If y'all could re-review.

@cmonr
Copy link
Contributor Author

cmonr commented Jun 13, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 13, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Jun 13, 2018

Exporter Build : SUCCESS

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

@cmonr cmonr mentioned this pull request Jun 14, 2018
Merged
@mbed-ci
Copy link

mbed-ci commented Jun 14, 2018

Test : SUCCESS

Build number : 2131
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7202/2131

@cmonr cmonr removed the needs: CI label Jun 15, 2018
@cmonr cmonr merged commit 8d88bb0 into master Jun 15, 2018
@0xc0170 0xc0170 deleted the cmonr-patch-error_handling-test branch June 27, 2018 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SenRamakri SenRamakri SenRamakri approved these changes

@0xc0170 0xc0170 0xc0170 left review comments

Assignees
No one assigned
Labels
release-version: 5.9.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cmonr @mbed-ci @0xc0170 @SenRamakri