Fix all clang-tidy warnings #340
Conversation
stojadin2701
force-pushed
the
combined-patches
branch
2 times, most recently
from
d0113a7 to
59888ca
Compare
stojadin2701
force-pushed
the
combined-patches
branch
from
59888ca to
053444d
Compare
stojadin2701
force-pushed
the
combined-patches
branch
from
053444d to
0769afb
Compare
- Release build on Windows - Debug build + linting on Ubuntu - Fail workflow if clang-tidy warnings are encountered
stojadin2701
force-pushed
the
combined-patches
branch
from
0769afb to
c0cecec
Compare
stojadin2701
force-pushed
the
combined-patches
branch
from
c0cecec to
6b9e679
Compare
stojadin2701
force-pushed
the
combined-patches
branch
from
6b9e679 to
2f69a08
Compare
stojadin2701
force-pushed
the
combined-patches
branch
from
2f69a08 to
50ee7a8
Compare
| size_t elem_sz = sizeof(rec_elem.u.gnss_info); | ||
| lfs_file_read(&lfs, &curr_file, (uint8_t *)&rec_elem.u.imu, elem_sz); | ||
| const size_t elem_sz = sizeof(rec_elem.u.gnss_info); | ||
| lfs_file_read(&lfs, &curr_file, reinterpret_cast<uint8_t *>(&rec_elem.u.imu), elem_sz); |
There was a problem hiding this comment.
Were those reinterpret_cast tested? Can you read from the file system?
There was a problem hiding this comment.
Yes. In general reinterpret_cast doesn't do anything fancy, it's more or less the same as doing a C-style cast, you only declare your intent with this. In this case, we are reinterpreting a pointer to an IMU object to a pointer to uint8_t, which we were already doing.
| @@ -224,16 +227,17 @@ void Telemetry::ParseRxMessage(packed_rx_msg_t* rx_payload) noexcept { | |||
|
|
|||
| PackTxMessage(tick_count, &gnss_data, &tx_payload, estimation_output); | |||
|
|
|||
| SendTxPayload((uint8_t*)&tx_payload, sizeof(packed_tx_msg_t)); | |||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) | |||
| SendTxPayload(reinterpret_cast<uint8_t*>(&tx_payload), sizeof(packed_tx_msg_t)); | |||
| int err = lfs_file_open(&lfs, &curr_file, filename, LFS_O_RDONLY); | ||
| if (err) { | ||
| const int err = lfs_file_open(&lfs, &curr_file, filename, LFS_O_RDONLY); | ||
| if (err < 0) { |
There was a problem hiding this comment.
From lfs documentation:
// Returns a negative error code on failure.
int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags);
So we should check for err < 0.
| LQmask = (1 << 0); | ||
| for (uint8_t i = 0; i < (sizeof(LQArray) / sizeof(LQArray[0])); i++) { | ||
| LQmask = (1U << 0U); | ||
| for (uint32_t i = 0; i < (sizeof(LQArray) / sizeof(LQArray[0])); i++) { |
There was a problem hiding this comment.
Because sizeof(LQArray) / sizeof(LQArray[0]) returns an uint32_t element. In reality this doesn't matter as from what I see LQArray's max size is 2, but I didn't want to change the semantics of the line in this patch.
| while (busyTransmitting) | ||
| ; | ||
| while (busyTransmitting) { | ||
| }; |
There was a problem hiding this comment.
The behavior is the same both with and without brackets, so it's as dangerous as it was :)
stojadin2701
force-pushed
the
combined-patches
branch
from
50ee7a8 to
faf3e7e
Compare
No description provided.