fix conversion warning for GCC/ARM_CM4F #653
Conversation
Signed-off-by: Vo Trung Chi <[email protected]>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #653 +/- ##
=======================================
Coverage 94.46% 94.46%
=======================================
Files 6 6
Lines 2422 2422
Branches 594 594
=======================================
Hits 2288 2288
Misses 85 85
Partials 49 49
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| @@ -51,7 +51,7 @@ | |||
| #define portLONG long | |||
| #define portSHORT short | |||
| #define portSTACK_TYPE uint32_t | |||
| #define portBASE_TYPE long | |||
| #define portBASE_TYPE uint32_t | |||
There was a problem hiding this comment.
I don't think it's ok to change this to an unsigned type - it is meant to be signed as per every other port and as it has always been. Changing it will probably cause pain for many users. If it is changed, which I advise strongly against, it will also need to be changed in all the ARMv7-M and ARMv8-M ports.
There was a problem hiding this comment.
portBASE_TYPE is actually a deprecated type, kept only so application code still builds. Recent code should be using BaseType_t.
There was a problem hiding this comment.
Thank you for your review and information. Currently, It caused the compile error because the warnings are treated as errors. What is your suggestion to fix this?
There was a problem hiding this comment.
Do you know which specific warning flag is giving you an error? The pedantic flag is something we've discussed removing in other repos in the past. If this is causing your issue - we might want to consider removing it.
There was a problem hiding this comment.
Also, which line is causing the error? Is it the kernel code or the application code?
There was a problem hiding this comment.
Sorry, this was -Wconversion flag in my application code.
There was a problem hiding this comment.
Thanks for taking the time to report back :)
| @@ -304,7 +304,7 @@ BaseType_t xPortStartScheduler( void ) | |||
|
|
|||
| #if ( configASSERT_DEFINED == 1 ) | |||
| { | |||
| volatile uint32_t ulOriginalPriority; | |||
| volatile uint8_t ulOriginalPriority; | |||
There was a problem hiding this comment.
This seems okay but it needs to be done across multiple ports. Do you want to do that too or would you prefer us to do that?
There was a problem hiding this comment.
Yes, I have created #658 . Thanks.
fix conversion warning for GCC/ARM_CM4F
Description
Test Steps
Checklist:
Related Issue
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.