-
Notifications
You must be signed in to change notification settings - Fork 3k
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 trivial compiler Warnings #13476
Conversation
@talorion, thank you for your changes. |
Nice! wanted to do the same :) |
https://github.com/orgs/ARMmbed/teams/mbed-os-maintainers : |
@talorion Would you be able to rebase on the latest master? |
@talorion Can you please rebase, the history looks incorrect (bad rebase?) |
Thanks for fixing these warnings 👍 I'll review once the history is correct (to make sure I am not reviewing unrelated changes). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs rebase as requested above
…Stack.cpp Co-authored-by: Hugues Kamba <[email protected]>
@@ -440,7 +440,7 @@ nsapi_size_or_error_t TELIT_ME310_CellularStack::socket_recvfrom_impl(CellularSo | |||
// read() should not fail | |||
success = false; | |||
} | |||
} else if (timer.read_ms() < ME310_SOCKET_TIMEOUT) { | |||
} else if (std::chrono::duration_cast<std::chrono::milliseconds>(timer.elapsed_time()) < socket_timeout) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The duration cast is not required here. You only need it when doing a precision-losing conversion.
Comparisons are always exact, regardless of the precision on either side. The less-precise thing gets converted to the more-precise thing.
} else if (std::chrono::duration_cast<std::chrono::milliseconds>(timer.elapsed_time()) < socket_timeout) { | |
} else if (timer.elapsed_time() < socket_timeout) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i changed it accordingly to your suggestions
@@ -372,7 +371,7 @@ UBLOX_AT_CellularStack::CellularSocket *UBLOX_AT_CellularStack::find_socket(int | |||
{ | |||
CellularSocket *socket = NULL; | |||
|
|||
for (unsigned int x = 0; (socket == NULL) && (x < _device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT)); x++) { | |||
for (intptr_t x = 0; (socket == NULL) && (x < _device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT)); x++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intptr_t
? That's not what you want here. (It's for storing pointers as integers).
You want size_t
or ptrdiff_t
, depending on whether you need it to be signed or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i changed it accordingly to your suggestions
@@ -60,7 +60,7 @@ extern "C" { // "pppos.h" is missing extern C | |||
/* Timeout to wait for PPP connection to be terminated | |||
* (LCP Terminate-Request is answered with Terminate-Ack) | |||
*/ | |||
#define PPP_TERMINATION_TIMEOUT 30000 | |||
#define PPP_TERMINATION_TIMEOUT 30s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't work - you don't have a using directive (and it's got a stray semicolon). Not used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i changed it accordingly to your suggestions
Please take into account Kevin's comment.
…Stack.cpp Co-authored-by: Kevin Bracey <[email protected]>
Co-authored-by: Kevin Bracey <[email protected]>
…ackRfPhyAT86RF215.cpp Co-authored-by: Kevin Bracey <[email protected]>
ping |
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Summary of changes
Fixed some trivial warnings with ARM_GCC and default profile.
Following warnings were fixed:
Impact of changes
Migration actions required
Documentation
None
Pull request type
Test results
Reviewers