-
Notifications
You must be signed in to change notification settings - Fork 7
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
ladislas/bugfix/cleanup warnings #1108
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1108 +/- ##
===========================================
+ Coverage 96.03% 96.06% +0.02%
===========================================
Files 138 138
Lines 3256 3251 -5
===========================================
- Hits 3127 3123 -4
+ Misses 129 128 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff list
|
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.
LGTM, just 3 questions
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.
- Validate on robot (Screen + BLE)
e3641e8
to
8e33950
Compare
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.
Please do not change code in extern
.
Except that, everything works fine ✅ (LekaOS seems to run normally on the robot)
0faddff
to
26f2f9c
Compare
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.
LGTM
@@ -161,7 +161,7 @@ TEST_F(CoreFontTest, displayNormalSentence) | |||
{ | |||
constexpr uint8_t buff_size = 128; | |||
char buff[buff_size] {}; | |||
auto text_length = sprintf(buff, "Some text"); | |||
auto text_length = snprintf(buff, buff_size, "Some text"); |
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.
Is "snprintf rather than sprintf" something to remind as a good practice ?
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.
yes, see:
https://www.gnu.org/software/libc/manual/html_node/Formatted-Output-Functions.html
Warning: The sprintf function can be dangerous because it can potentially output more characters than can fit in the allocation size of the string s. Remember that the field width given in a conversion specification is only a minimum value.
The snprintf function is similar to sprintf, except that the size argument specifies the maximum number of characters to produce.
The return value is the number of characters which would be generated for the given input, excluding the trailing null. If this value is greater than or equal to size, not all characters from the result have been stored in s. You should try again with a bigger output string.
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.
yes, see:
https://www.gnu.org/software/libc/manual/html_node/Formatted-Output-Functions.html
Warning: The sprintf function can be dangerous because it can potentially output more characters than can fit in the allocation size of the string s. Remember that the field width given in a conversion specification is only a minimum value.
The snprintf function is similar to sprintf, except that the size argument specifies the maximum number of characters to produce.
The return value is the number of characters which would be generated for the given input, excluding the trailing null. If this value is greater than or equal to size, not all characters from the result have been stored in s. You should try again with a bigger output string.
2a24966
to
73652c9
Compare
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.
Some questions and precisions.
I add a commit to suggest a precision.
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.
Looks very great! Thank you for the job done~
0923d97
to
0740937
Compare
… different signedness
…g' but the argument has type 'uint32_t' Using <cinttypes> and PRI... format specifiers allows code to compile without warning on both arm-none-eabi-gcc and clang/gcc on host machine. See for more information: - https://en.cppreference.com/w/cpp/io/c/fprintf - https://en.cppreference.com/w/cpp/header/cinttypes
…n-defined + NOLINT reinterpret_cast
- Do not use array subscript when the index is not an integer constant expression - Constructor does not initialize these fields: convertMCUBlocks - Performing an implicit widening conversion to type 'size_t' (aka 'unsigned long') of a multiplication performed in type 'unsigned int' - Performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int' - xx is a magic number; consider replacing it with a named constant - Missing username/bug in TODO - Do not use C-style cast to convert between unrelated types - Result of multiplication in type 'unsigned int' is used as a pointer offset after an implicit widening conversion to type 'size_t' - Do not use array subscript when the index is not an integer constant expression
- warning: missing username/bug in TODO - warning: 4000 is a magic number; consider replacing it with a named constant
… provided for compatibility reasons only
0740937
to
ca474ed
Compare
SonarCloud Quality Gate failed. |
this PR fixes some (a lot) of sonarcloud, clang-tidy and compiler (gcc/clang) warnings/errors.